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.

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

Review and enforce CRD Validation #2993

Open
6 tasks
pjbgf opened this issue Aug 17, 2022 · 5 comments
Open
6 tasks

Review and enforce CRD Validation #2993

pjbgf opened this issue Aug 17, 2022 · 5 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@pjbgf
Copy link
Member

pjbgf commented Aug 17, 2022

In the run-up to maturing flux APIs, we should review all CRD validation and ensure that they align with expected input.

Controllers:

  • source-controller
  • kustomize-controller
  • helm-controller
  • notification-controller
  • image-reflector-controller
  • image-automation-controller

Additional points to consider:

@pjbgf pjbgf added this to the GA milestone Aug 17, 2022
@pjbgf pjbgf added the good first issue Good for newcomers label Aug 17, 2022
@Santosh1176
Copy link
Contributor

Hey @pjbgf I would like work on this. I am not sure about couple of points, can you please clarify:

  • Point 1: The validation and tests needs to be added to ValueKey and TargetPath fields only?
  • Point 2: Got it
  • Point3: max length and patterns be defined on reference types from pkg linked in Point 4?

@pjbgf pjbgf assigned pjbgf and Santosh1176 and unassigned pjbgf Aug 18, 2022
@pjbgf
Copy link
Member Author

pjbgf commented Aug 18, 2022

@Santosh1176 here's more info:

  1. The PR was just an example on how to implement the validation, as in added via +kubebuilder:validation and then tested with testenv. We need to review each CRD for all controllers, and make sure that all fields are validated.
  2. OK
  3. The validation will depend on each field at hand, so may need some research on how that field is being used, and what rules it has to abide by.
  4. Those reference types at the moment only have +required, they lack max length and pattern validation. In some CRD we may be using other types instead of them to refer to Kubernetes objects, when that's the case we must change that.

@Santosh1176
Copy link
Contributor

@pjbgf Any reference/guidance for max and min length properties for strings fields?

@pjbgf pjbgf added the help wanted Extra attention is needed label Aug 23, 2022
@pjbgf
Copy link
Member Author

pjbgf commented Aug 23, 2022

@Santosh1176 unfortunately this will be on a case-by-case basis. Feel free to propose some validation for one of the CRDs and during the review we will provide you with some feedback.

@pjbgf pjbgf mentioned this issue Aug 26, 2022
11 tasks
@snebel29
Copy link
Contributor

snebel29 commented Sep 6, 2022

@Santosh1176 I am interested contributing to this issue as well, have you made any progress since?

What about I take one of those task item? for example, the notification controller?

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: In Progress
Development

No branches or pull requests

3 participants