Update validation schema#32
Conversation
jalvarezit
changed the title
…ost' and adjust validation schema
There was a problem hiding this comment.
Pull request overview
Updates the registry validation/mapping to support a url field in addition to the existing registry field, and renames internal usage from RegistryHost to Url.
Changes:
- Extend the JSON schema to accept
urlas an alternative toregistry. - Rename the
Registrystruct field fromRegistryHosttoUrland update call sites/tests accordingly. - Add custom YAML/JSON unmarshaling on
Registryto accept eitherregistryorurl.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| validations.go | Schema updated to accept url or registry via oneOf. |
| types.go | RegistryHost replaced with Url; adds custom YAML/JSON unmarshal logic. |
| io.go | Switch duplicate detection / lookup to use registry.Url; adds skip logic for empty auth_strategy. |
| multilogin.go | Use registry.Url for Azure/generic/ghcr auth flows. |
| validations_test.go | Update schema validation tests to use Url field. |
| io_test.go | Update parser tests to assert Url field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d adjust required fields
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestRegistriesParser(t *testing.T) { | ||
|
|
||
| registries := parseRegistriesFromDir("./test/fixtures/registries") | ||
|
|
||
| fmt.Printf("Registries: %v\n", registries[0].RegistryHost) | ||
| fmt.Printf("Registries: %v\n", registries[0].Url) | ||
|
|
There was a problem hiding this comment.
Tests only cover parsing legacy registry: fixtures. Since Registry now supports url: as well (and intends to forbid specifying both), add coverage for a fixture or inline YAML using url: and for the “both keys present” error case so this migration behavior is protected.
| if reg, ok := aux["registry"].(string); ok { | ||
| r.Url = reg | ||
| } else if url, ok := aux["url"].(string); ok { | ||
| r.Url = url | ||
| } |
There was a problem hiding this comment.
UnmarshalYAML accepts both registry and url, but if both keys are present it silently prefers registry and discards url. That means the schema’s oneOf mutual-exclusion rule won’t actually be enforced (the validator never sees both). If the intent is “exactly one of these keys”, detect this during YAML unmarshal and return an error when both are set.
…' in types and tests
* fix: update validation schema to include 'url' and adjust required fields * refactor: update registry handling to use 'url' instead of 'registryHost' and adjust validation schema * fix: update registry validation to use 'url' instead of 'registry' and adjust required fields * update oauth oci * fix: update registry field to use 'RegistryHost' instead of 'Registry' in types and tests * fix: correct formatting in validation schema definition * fix: update validation schema to remove unnecessary required fields * fix: update validation schema to allow null values for image_types and base_paths
related to https://github.com/prefapp/features/issues/815