Skip to content

CRD validation ratcheting: userInterface #4198

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18,631 changes: 18,631 additions & 0 deletions config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions internal/controller/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/crunchydata/postgres-operator/internal/logging"
v1 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1"
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
)

Expand All @@ -34,6 +35,9 @@ func init() {
if err := v1beta1.AddToScheme(Scheme); err != nil {
panic(err)
}
if err := v1.AddToScheme(Scheme); err != nil {
panic(err)
}
if err := volumesnapshotv1.AddToScheme(Scheme); err != nil {
panic(err)
}
Expand Down
30 changes: 30 additions & 0 deletions internal/testing/require/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"golang.org/x/tools/go/packages"
"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/version"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
Expand Down Expand Up @@ -73,6 +75,34 @@ func Kubernetes(t TestingT) client.Client {
return cc
}

// KubernetesAtLeast is the same as [Kubernetes] but also calls t.Skip when
// the connected Kubernetes API is earlier than minVersion, like "1.28" or "1.27.7".
func KubernetesAtLeast(t TestingT, minVersion string) client.Client {
t.Helper()

expectedVersion, err := version.ParseGeneric(minVersion)
assert.NilError(t, err)

// Start or connect to Kubernetes
env, cc := kubernetes3(t)

dc, err := discovery.NewDiscoveryClientForConfig(env.Config)
assert.NilError(t, err)

serverInfo, err := dc.ServerVersion()
assert.NilError(t, err)

serverVersion, err := version.ParseGeneric(serverInfo.GitVersion)
assert.NilError(t, err)

if serverVersion.LessThan(expectedVersion) {
t.Log("Kubernetes version", serverVersion, "is before", expectedVersion)
t.SkipNow()
}

return cc
}

// Kubernetes2 is the same as [Kubernetes] but also returns a copy of the client
// configuration.
func Kubernetes2(t TestingT) (*rest.Config, client.Client) {
Expand Down
99 changes: 99 additions & 0 deletions internal/testing/validation/postgrescluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
"github.com/crunchydata/postgres-operator/internal/testing/require"
v1 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1"
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
)

Expand Down Expand Up @@ -519,3 +520,101 @@ func TestPostgresUserOptions(t *testing.T) {
assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll))
})
}

func TestPostgresUserInterfaceAcrossVersions(t *testing.T) {
ctx := context.Background()
cc := require.Kubernetes(t)
t.Parallel()

namespace := require.Namespace(t, cc)

base := v1beta1.NewPostgresCluster()
// Start with a bunch of required fields.
base.Namespace = namespace.Name
base.Name = "postgres-pgadmin"
require.UnmarshalInto(t, &base.Spec, `{
userInterface: {
pgAdmin: {
dataVolumeClaimSpec: {
accessModes: [ReadWriteOnce],
resources: { requests: { storage: 1Mi } },
},
},
},
postgresVersion: 16,
backups: {
Copy link
Member

Choose a reason for hiding this comment

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

🤔 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.

pgbackrest: {
repos: [{ name: repo1 }],
},
},
instances: [{
dataVolumeClaimSpec: {
accessModes: [ReadWriteOnce],
resources: { requests: { storage: 1Mi } },
},
}],
}`)

v1base := v1.NewPostgresCluster()
// Start with a bunch of required fields.
v1base.Namespace = namespace.Name
v1base.Name = "postgres-pgadmin"
require.UnmarshalInto(t, &v1base.Spec, `{
userInterface: {
pgAdmin: {
dataVolumeClaimSpec: {
accessModes: [ReadWriteOnce],
resources: { requests: { storage: 1Mi } },
},
},
},
postgresVersion: 16,
backups: {
Copy link
Member

Choose a reason for hiding this comment

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

🤔 🤔 I'll think about making it a pointer on the new struct!

pgbackrest: {
repos: [{ name: repo1 }],
},
},
instances: [{
dataVolumeClaimSpec: {
accessModes: [ReadWriteOnce],
resources: { requests: { storage: 1Mi } },
},
}],
}`)

t.Run("v1beta1 is valid with pgadmin", func(t *testing.T) {
assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll),
"expected this base cluster to be valid")
})
t.Run("v1 is invalid with pgadmin", func(t *testing.T) {
assert.ErrorContains(t, cc.Create(ctx, v1base.DeepCopy(), client.DryRunAll),
"userInterface not available in v1")
})

t.Run("v1 is valid with pgadmin but only if unchanged from v1beta1", func(t *testing.T) {
// Validation ratcheting is enabled starting in Kubernetes 1.30
require.KubernetesAtLeast(t, "1.30")

// A v1 that has been updated from a v1beta1 with no change to the userInterface is valid
assert.NilError(t, cc.Create(ctx, base),
"expected this base cluster to be valid")
v1base.ResourceVersion = base.ResourceVersion
assert.NilError(t, cc.Update(ctx, v1base),
"expected this v1 cluster to be a valid update")

// But will not be valid if there's a change to the userInterface
require.UnmarshalInto(t, &v1base.Spec, `{
userInterface: {
pgAdmin: {
dataVolumeClaimSpec: {
accessModes: [ReadWriteOnce, ReadWriteMany],
resources: { requests: { storage: 2Mi } },
},
},
},
}`)

assert.ErrorContains(t, cc.Update(ctx, v1base),
"userInterface not available in v1")
Comment on lines +617 to +618
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻 I like this test!

})
}
24 changes: 24 additions & 0 deletions pkg/apis/postgres-operator.crunchydata.com/v1/groupversion_info.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2021 - 2025 Crunchy Data Solutions, Inc.
//
// SPDX-License-Identifier: Apache-2.0

// package v1 contains API Schema definitions for the postgres-operator v1beta1 API group
// +kubebuilder:object:generate=true
// +groupName=postgres-operator.crunchydata.com
package v1

import (
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/scheme"
)

var (
// GroupVersion is group version used to register these objects
GroupVersion = schema.GroupVersion{Group: "postgres-operator.crunchydata.com", Version: "v1"}

// SchemeBuilder is used to add go types to the GroupVersionKind scheme
SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}

// AddToScheme adds the types in this group-version to the given scheme.
AddToScheme = SchemeBuilder.AddToScheme
)
Loading
Loading