Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Schema and strict validation radixconfig #95

Merged
merged 5 commits into from
Jun 3, 2024
Merged

Schema and strict validation radixconfig #95

merged 5 commits into from
Jun 3, 2024

Conversation

Richard87
Copy link
Contributor

@Richard87 Richard87 commented May 27, 2024 •
edited
Loading

Todo:

  • Update operator with keda changes
Validating /home/richard/projects/tests/rihag-edc23-radix-1/radixconfig.yaml
 - /spec/components/1/horizontalScaling/triggers/2/cron/start: does not match pattern '^((((\\d+,)+\\d+|(\\d+(\\/|-)\\d+)|\\d+|\\*) ?){5})$'
 - strict test failed: error unmarshaling JSON: while decoding JSON: json: unknown field "test"
RadixConfig is invalid

@Richard87 Richard87 self-assigned this May 28, 2024
@Richard87 Richard87 changed the title validate against crd and strictly Schema and strict validation radixconfig May 28, 2024
@Richard87 Richard87 requested a review from nilsgstrabo May 31, 2024 12:38
@Richard87 Richard87 marked this pull request as ready for review May 31, 2024 12:38
nilsgstrabo
nilsgstrabo requested changes Jun 3, 2024
View reviewed changes
Copy link
Contributor

@nilsgstrabo nilsgstrabo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • go.mod is using go1.22, but Dockerfile nad github actions still reference go1.21.
  • should we update golangci-lint to newer version?
  • should we update goreleaser to newer version?

cmd/validateRadixConfig.go Outdated
@@ -87,6 +168,7 @@ func init() {
validateCmd.AddCommand(validateRadixConfigCmd)
validateRadixConfigCmd.Flags().StringP(flagnames.ConfigFile, "f", "radixconfig.yaml", "Name of the radixconfig file. Defaults to radixconfig.yaml in current directory")
validateRadixConfigCmd.Flags().BoolP(flagnames.Print, "p", false, "Print parsed config file")
validateRadixConfigCmd.Flags().StringP(flagnames.Schema, "s", "https://raw.githubusercontent.com/equinor/radix-operator/release/json-schema/radixapplication.json", "Validate against schema")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a shorthand for schema? Most users will never use it, and the more shorthands we add, the more chance of a conflict with other flags later.
Also, add some info about what values this flag accepts: url (https://) or file (file://)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enig! la inn Validate against schema. http://, file:// or path is supported, og installerte samme versjon som Radix-API, og på samme måte :)

@Richard87 Richard87 merged commit 103a074 into master Jun 3, 2024
2 checks passed
@Richard87 Richard87 deleted the strict-validation branch June 3, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nilsgstrabo nilsgstrabo nilsgstrabo approved these changes

Assignees

@Richard87 Richard87

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Richard87 @nilsgstrabo