OLS-1435: add validation webhook for OLSConfig CRD

[raptorsun]

Description

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up dependent library

Related Tickets & Documents

• Related Issue #
• Closes #

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.
• If it is a core feature, I have added thorough tests.

Testing

• Please provide detailed steps to perform tests related to this code change.
• How were the fix/results from this change verified? Please provide relevant screenshots or results.

[openshift-ci-robot]

@raptorsun: This pull request references OLS-1435 which is a valid jira issue.

In response to this:

Description

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up dependent library

Related Tickets & Documents

• Related Issue #
• Closes #

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.
• If it is a core feature, I have added thorough tests.

Testing

• Please provide detailed steps to perform tests related to this code change.
• How were the fix/results from this change verified? Please provide relevant screenshots or results.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign raptorsun for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[bparees]

@raptorsun did you look at using CRD schema validation rules to do this instead of a webhook? I'm pretty sure it's possible to define schema rules for most, if not all, of these checks and avoid running/managing the webhook (not to mention the webhook becomes a point of failure since the CR instances can't be managed if the webhook is down for some reason):

https://kubernetes.io/blog/2022/09/23/crd-validation-rules-beta/

even if you don't want to use the beta feature (CEL), there's also validation that can be done on things like field name (for the cluster name requirement):
https://book.kubebuilder.io/reference/markers/crd-validation

[raptorsun]

/retest

[raptorsun]

@raptorsun did you look at using CRD schema validation rules to do this instead of a webhook? I'm pretty sure it's possible to define schema rules for most, if not all, of these checks and avoid running/managing the webhook (not to mention the webhook becomes a point of failure since the CR instances can't be managed if the webhook is down for some reason):

https://kubernetes.io/blog/2022/09/23/crd-validation-rules-beta/

even if you don't want to use the beta feature (CEL), there's also validation that can be done on things like field name (for the cluster name requirement): https://book.kubebuilder.io/reference/markers/crd-validation

Yes, I have checked the validation tools provided by kubebuilder. CRD validation rules works well when all the required information comes from the object itself. Last year we have investigated several config problems reported by users, many of which leads to a reference to inexistent secret, configmap. CRD validation rules cannot verify reffered resources so I'd like to put a validation webhook here to prevent users mistakenly change the OLSConfig with reference to inexistent resources. This PR starts with the token secret. In future we will add reference to configmaps such as those containing TLS certs, too.

[bparees]

CRD validation rules cannot verify reffered resources so I'd like to put a validation webhook here to prevent users mistakenly change the OLSConfig with reference to inexistent resources. This PR starts with the token secret. In future we will add reference to configmaps such as those containing TLS certs, too.

If someone deletes the referenced configmap/secret, does the operator reconciliation flag that in status the next time it reconciles the olsconfig object?

[raptorsun]

CRD validation rules cannot verify reffered resources so I'd like to put a validation webhook here to prevent users mistakenly change the OLSConfig with reference to inexistent resources. This PR starts with the token secret. In future we will add reference to configmaps such as those containing TLS certs, too.

If someone deletes the referenced configmap/secret, does the operator reconciliation flag that in status the next time it reconciles the olsconfig object?

yes, errors during reconciliation is put into the status of OLSConfig.

[raptorsun]

leave these code blocks here in case we need mutation webhook in future

[raptorsun]

/retest

[raptorsun]

bundle is still using old operator image, we need merge this PR first and then quickly update the operator image in the bundle to deblock the integrations tests:

• Red Hat Konflux / service-e2e-tests-417 / ols-bundle
• Red Hat Konflux / operator-e2e-tests-417 / ols-bundle

[JoaoFula]

/lgtm

[openshift-ci]

New changes are detected. LGTM label has been removed.

[openshift-ci]

@raptorsun: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.