CRD validation ratcheting: userInterface #4198
Conversation
pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types_conversion.go
Outdated
Show resolved
Hide resolved
| func TestPostgresClusterWebhooks(t *testing.T) { | ||
| var _ webhook.Defaulter = new(PostgresCluster) | ||
| } |
There was a problem hiding this comment.
I noticed in a Dependabot PR that this doesn't compile with newer controller-runtime:
golangci-lint: pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types_test.go#L18
undefined: webhook.Defaulter (typecheck)
⛏️ What do you think of removing this now, in both apiVersions, as we're considering ratcheting over webhooks?
There was a problem hiding this comment.
I actually removed this file locally since we didn't make any changes, but now I'm waffling. Ah well, removing for now.
benjaminjb
force-pushed
the
benjb/crd-validation-ratcheting
branch
from
15f72a7 to
8c2ad0e
Compare
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // +kubebuilder:validation:Schemaless | ||
| // +kubebuilder:validation:Type=object | ||
| // +kubebuilder:validation:XValidation:rule="type(self) == null_type || self == oldSelf", message="DynamicConfiguration not available in v1, please use ..." |
There was a problem hiding this comment.
👍🏻 Ah! oldSelf means no K8s ratcheting, and self == oldSelf sounds similar to the built-in ratcheting.
| // The PGBackRest field is incompatible with the PostgresCluster field: only one | ||
| // data source can be used for pre-populating a new PostgreSQL cluster | ||
| // +optional | ||
| // +kubebuilder:validation:XValidation:rule="!has(self.repo.volume)", message="Only S3, GCS or Azure repos can be used as a pgBackRest data source.", fieldPath=".repo" |
There was a problem hiding this comment.
🤔 I wish there were a way to define these rules over multiple lines.
| @@ -0,0 +1,740 @@ | |||
| // Copyright 2021 - 2025 Crunchy Data Solutions, Inc. | |||
There was a problem hiding this comment.
🤔 Looks like the copy/re-use goes file-wise; that works for me. We can arrange things into smaller files if/as we find it necessary.
| }, | ||
| }, | ||
| postgresVersion: 16, | ||
| backups: { |
There was a problem hiding this comment.
🤔 I remember now that I tried turning backups into a pointer so we could omit it in these tests, and it sent me down some long hole.
| }, | ||
| }, | ||
| postgresVersion: 16, | ||
| backups: { |
There was a problem hiding this comment.
🤔 🤔 I'll think about making it a pointer on the new struct!
| assert.ErrorContains(t, cc.Update(ctx, v1base), | ||
| "userInterface not available in v1") |
- Add kubebuilder markers to v1beta1 to mark as storage version - Add v1 files for changed version (postgrescluster) - Add rule to disallow userInterface - Add v1 conversion test, runtime scheme - Add a helper for kube version tests - Remove ref to webhook in v1beta1 Issues: [PGO-2524]
benjaminjb
force-pushed
the
benjb/crd-validation-ratcheting
branch
from
8c2ad0e to
b8b9d49
Compare
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
What is the new behavior (if this is a feature change)?
This lays the groundwork for a v1 of the CRD; with a conversion type of None and validation ratcheting to handle differences between versions.
Other Information:
Issues: [PGO-2524]