Skip to content

chore: Remove validation from reconcile loop#7

Merged
TeddyAndrieux merged 2 commits intomainfrom
improvement/remove-validate-from-reconcile
Nov 12, 2025
Merged

chore: Remove validation from reconcile loop#7
TeddyAndrieux merged 2 commits intomainfrom
improvement/remove-validate-from-reconcile

Conversation

@TeddyAndrieux
Copy link
Contributor

No description provided.

@TeddyAndrieux TeddyAndrieux requested a review from a team as a code owner November 7, 2025 13:04
if ces.Image == nil {
ces.Image = &ImageSpec{}
}
ces.Image.withDefaults()

Choose a reason for hiding this comment

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

Normally no default needed as the field is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer mandatory

(I'm not really agree with this statement, it's not because a field is mandatory that you do not have "sub fields" with defaults that should be set)

Choose a reason for hiding this comment

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

"mouais", in that case, you validate instead of defaulting
But no way, the field is no more mandatory

// validationManagedCRL validates the ManagedCRL fields.
func validationManagedCRL(logger logr.Logger, ctx context.Context, c client.Client, managedcrl *crloperatorv1alpha1.ManagedCRL) error {
managedcrl.WithDefaults()
if err := managedcrl.Validate(); err != nil {

Choose a reason for hiding this comment

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

Why ? validation does not expected some default fields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The withDefaults as been moved to the Validate function directly

Base automatically changed from improvement/add-validation-webhook to main November 12, 2025 09:39
@TeddyAndrieux TeddyAndrieux force-pushed the improvement/remove-validate-from-reconcile branch 2 times, most recently from b4096b2 to 713a0a8 Compare November 12, 2025 10:38
Signed-off-by: Teddy Andrieux <teddy.andrieux@scality.com>
Signed-off-by: Teddy Andrieux <teddy.andrieux@scality.com>
@TeddyAndrieux TeddyAndrieux force-pushed the improvement/remove-validate-from-reconcile branch from 713a0a8 to 6775a3b Compare November 12, 2025 10:42
Copy link

Choose a reason for hiding this comment

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

ok for me

@TeddyAndrieux TeddyAndrieux merged commit e6ed7d7 into main Nov 12, 2025
5 checks passed
@TeddyAndrieux TeddyAndrieux deleted the improvement/remove-validate-from-reconcile branch November 12, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants