From 2d6b9df8fe70da6ed067796f5417164857f564b9 Mon Sep 17 00:00:00 2001 From: Nicolas Bigler Date: Thu, 26 Sep 2024 16:09:47 +0200 Subject: [PATCH] Fix dependency issue when using encrypted PVCs for VSHNPostgreSQL There was a dependency loop that prevented the creation of VSHNPostgreSQL instances that use encrypted PVCs: * The SGCluster object can't be created before the luks secret for the PVC is generated * The luks secret can't be created without getting the instance count of the cluster object This has been resolved by relying on the instance count from the composite and also waiting for the luks secrets to be present before attempting to access the sgcluster object to be able to override the storageClass for the PVCs Signed-off-by: Nicolas Bigler --- .../functions/vshnkeycloak/deploy.go | 2 +- .../functions/vshnnextcloud/deploy.go | 2 +- .../functions/vshnpostgres/encrypted_pvc.go | 24 ++++++++++------- .../vshnpostgres/encrypted_pvc_test.go | 19 +++++++------- pkg/comp-functions/runtime/function_mgr.go | 26 +++++++++++++++---- .../enc_pvc/03-GivenEncryptionParams.yaml | 1 + ...3-GivenEncryptionParamsExistingSecret.yaml | 3 ++- 7 files changed, 50 insertions(+), 27 deletions(-) diff --git a/pkg/comp-functions/functions/vshnkeycloak/deploy.go b/pkg/comp-functions/functions/vshnkeycloak/deploy.go index 1d95478ea..0afe54ed0 100644 --- a/pkg/comp-functions/functions/vshnkeycloak/deploy.go +++ b/pkg/comp-functions/functions/vshnkeycloak/deploy.go @@ -91,7 +91,7 @@ func DeployKeycloak(ctx context.Context, comp *vshnv1.VSHNKeycloak, svc *runtime }, } - ready, err := svc.WaitForDependenciesWithConnectionDetails(comp.GetName(), resourceCDMap) + ready, err := svc.WaitForObservedDependenciesWithConnectionDetails(comp.GetName(), resourceCDMap) if err != nil { // We're returning a fatal here, so in case something is wrong we won't delete anything by mistake. return runtime.NewFatalResult(err) diff --git a/pkg/comp-functions/functions/vshnnextcloud/deploy.go b/pkg/comp-functions/functions/vshnnextcloud/deploy.go index 552e14238..92c7b1724 100644 --- a/pkg/comp-functions/functions/vshnnextcloud/deploy.go +++ b/pkg/comp-functions/functions/vshnnextcloud/deploy.go @@ -87,7 +87,7 @@ func DeployNextcloud(ctx context.Context, comp *vshnv1.VSHNNextcloud, svc *runti }, } - ready, err := svc.WaitForDependenciesWithConnectionDetails(comp.GetName(), resourceCDMap) + ready, err := svc.WaitForObservedDependenciesWithConnectionDetails(comp.GetName(), resourceCDMap) if err != nil { // We're returning a fatal here, so in case something is wrong we won't delete anything by mistake. return runtime.NewFatalResult(err) diff --git a/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc.go b/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc.go index 8875b3555..ffa43638d 100644 --- a/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc.go +++ b/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc.go @@ -41,6 +41,19 @@ func AddPvcSecret(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runtime log.Info("Adding secret to composite") + pods := comp.GetInstances() + + for i := 0; i < pods; i++ { + result := writeLuksSecret(svc, log, comp, i) + if result != nil { + return result + } + ready := svc.WaitForDesiredDependencies(comp.GetName(), fmt.Sprintf("%s-luks-key-%d", comp.Name, i)) + if !ready { + return runtime.NewWarningResult("luks secret not yet ready") + } + } + cluster := &stackgresv1.SGCluster{} err = svc.GetDesiredKubeObject(cluster, "cluster") if err != nil { @@ -54,19 +67,10 @@ func AddPvcSecret(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runtime return runtime.NewFatalResult(fmt.Errorf("Cannot edit SGCluster object: %w", err)) } - pods := cluster.Spec.Instances - - for i := 0; i < pods; i++ { - result := writeLuksSecret(ctx, svc, log, comp, i) - if result != nil { - return result - } - } - return nil } -func writeLuksSecret(ctx context.Context, svc *runtime.ServiceRuntime, log logr.Logger, comp *vshnv1.VSHNPostgreSQL, i int) *xfnproto.Result { +func writeLuksSecret(svc *runtime.ServiceRuntime, log logr.Logger, comp *vshnv1.VSHNPostgreSQL, i int) *xfnproto.Result { // luksSecretResourceName is the resource name defined in the composition // This name is different from metadata.name of the same resource // The value is hardcoded in the composition for each resource and due to crossplane limitation diff --git a/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc_test.go b/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc_test.go index 45d69868c..d74ce38c7 100644 --- a/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc_test.go +++ b/pkg/comp-functions/functions/vshnpostgres/encrypted_pvc_test.go @@ -12,6 +12,7 @@ import ( xkube "github.com/vshn/appcat/v4/apis/kubernetes/v1alpha2" stackgresv1 "github.com/vshn/appcat/v4/apis/stackgres/v1" vshnv1 "github.com/vshn/appcat/v4/apis/vshn/v1" + "github.com/vshn/appcat/v4/pkg/comp-functions/runtime" v1 "k8s.io/api/core/v1" "k8s.io/utils/pointer" "sigs.k8s.io/yaml" @@ -70,7 +71,7 @@ func TestGivenEncrypedPvcThenExpectOutput(t *testing.T) { r := AddPvcSecret(ctx, &vshnv1.VSHNPostgreSQL{}, svc) - assert.Nil(t, r) + assert.Equal(t, r, runtime.NewWarningResult("luks secret not yet ready")) comp := &vshnv1.VSHNPostgreSQL{} @@ -83,31 +84,31 @@ func TestGivenEncrypedPvcThenExpectOutput(t *testing.T) { s := &v1.Secret{} assert.NoError(t, yaml.Unmarshal(kubeObject.Spec.ForProvider.Manifest.Raw, s)) assert.NotEmpty(t, s.Data["luksKey"]) - - cluster := &stackgresv1.SGCluster{} - assert.NoError(t, svc.GetDesiredKubeObject(cluster, "cluster")) - assert.Equal(t, pointer.String("ssd-encrypted"), cluster.Spec.Pods.PersistentVolume.StorageClass) }) t.Run("GivenEncryptionEnabledExistingSecret_ThenExpectOutput", func(t *testing.T) { - iof := commontest.LoadRuntimeFromFile(t, "vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml") + svc := commontest.LoadRuntimeFromFile(t, "vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml") - r := AddPvcSecret(ctx, &vshnv1.VSHNPostgreSQL{}, iof) + r := AddPvcSecret(ctx, &vshnv1.VSHNPostgreSQL{}, svc) assert.Nil(t, r) comp := &vshnv1.VSHNPostgreSQL{} - assert.NoError(t, iof.GetObservedComposite(comp)) + assert.NoError(t, svc.GetObservedComposite(comp)) resName := comp.Name + "-luks-key-0" kubeObject := &xkube.Object{} - assert.NoError(t, iof.GetDesiredComposedResourceByName(kubeObject, resName)) + assert.NoError(t, svc.GetDesiredComposedResourceByName(kubeObject, resName)) s := &v1.Secret{} assert.NoError(t, yaml.Unmarshal(kubeObject.Spec.ForProvider.Manifest.Raw, s)) assert.NotEmpty(t, s.Data["luksKey"]) + + cluster := &stackgresv1.SGCluster{} + assert.NoError(t, svc.GetDesiredKubeObject(cluster, "cluster")) + assert.Equal(t, pointer.String("ssd-encrypted"), cluster.Spec.Pods.PersistentVolume.StorageClass) }) } diff --git a/pkg/comp-functions/runtime/function_mgr.go b/pkg/comp-functions/runtime/function_mgr.go index 2f12bbd07..87f28a58c 100644 --- a/pkg/comp-functions/runtime/function_mgr.go +++ b/pkg/comp-functions/runtime/function_mgr.go @@ -995,11 +995,11 @@ func (s *ServiceRuntime) areResourcesReady(names []string) bool { return true } -// WaitForDependencies takes two arguments, the name of the main resource, which should be deployed after the dependencies. +// WaitForObservedDependencies takes two arguments, the name of the main resource, which should be deployed after the dependencies. // It also takes a list of names for objects to depend on. It does NOT deploy any objects, but check for their existence. // If true is returned it is safe to continue with adding your main object to the desired resources. // If the main resource already exists in the observed state it will always return true. -func (s *ServiceRuntime) WaitForDependencies(mainResource string, dependencies ...string) bool { +func (s *ServiceRuntime) WaitForObservedDependencies(mainResource string, dependencies ...string) bool { if _, ok := s.req.Observed.Resources[mainResource]; ok { return true } @@ -1011,17 +1011,33 @@ func (s *ServiceRuntime) WaitForDependencies(mainResource string, dependencies . return true } -// WaitForDependenciesWithConnectionDetails does the same as WaitForDependencies but additionally also checks the given list of fields against the +// WaitForDesiredDependencies takes two arguments, the name of the main resource, which should be deployed after the dependencies. +// It also takes a list of names for objects to depend on. It does NOT deploy any objects, but check for their existence. +// If true is returned it is safe to continue with adding your main object to the desired resources. +// If the main resource already exists in the observed state it will always return true. +func (s *ServiceRuntime) WaitForDesiredDependencies(mainResource string, dependencies ...string) bool { + if _, ok := s.req.Desired.Resources[mainResource]; ok { + return true + } + + if !s.areResourcesReady(dependencies) { + return false + } + + return true +} + +// WaitForObservedDependenciesWithConnectionDetails does the same as WaitForDependencies but additionally also checks the given list of fields against the // available connection details. // objectCDMap should contain a map where the key is the name of the dependeny and the string slice the necessary connection detail fields. -func (s *ServiceRuntime) WaitForDependenciesWithConnectionDetails(mainResource string, objectCDMap map[string][]string) (bool, error) { +func (s *ServiceRuntime) WaitForObservedDependenciesWithConnectionDetails(mainResource string, objectCDMap map[string][]string) (bool, error) { // If the main resource already exists we're done here if _, ok := s.req.Observed.Resources[mainResource]; ok { return true, nil } for dep, cds := range objectCDMap { - ready := s.WaitForDependencies(mainResource, dep) + ready := s.WaitForObservedDependencies(mainResource, dep) if !ready { return false, nil } diff --git a/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParams.yaml b/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParams.yaml index b346f678b..bcfdca7eb 100644 --- a/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParams.yaml +++ b/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParams.yaml @@ -88,6 +88,7 @@ observed: name: vshnpostgres.vshn.appcat.vshn.io-ce52f13 compositionUpdatePolicy: Automatic parameters: + instances: 1 encryption: enabled: true status: diff --git a/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml b/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml index 047a19d50..be543b6a7 100644 --- a/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml +++ b/test/functions/vshn-postgres/enc_pvc/03-GivenEncryptionParamsExistingSecret.yaml @@ -88,12 +88,13 @@ observed: name: vshnpostgres.vshn.appcat.vshn.io-ce52f13 compositionUpdatePolicy: Automatic parameters: + instances: 1 encryption: enabled: true status: instanceNamespace: my-psql resources: - psql-luks-key: + psql-luks-key-0: resource: apiVersion: kubernetes.crossplane.io/v1alpha1 kind: Object