Skip to content
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

Fix dependency issue when using encrypted PVCs for VSHNPostgreSQL #236

Merged
merged 1 commit into from
Sep 27, 2024
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
2 changes: 1 addition & 1 deletion pkg/comp-functions/functions/vshnkeycloak/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/comp-functions/functions/vshnnextcloud/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 14 additions & 10 deletions pkg/comp-functions/functions/vshnpostgres/encrypted_pvc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
19 changes: 10 additions & 9 deletions pkg/comp-functions/functions/vshnpostgres/encrypted_pvc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}

Expand All @@ -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)
})

}
26 changes: 21 additions & 5 deletions pkg/comp-functions/runtime/function_mgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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) {
TheBigLee marked this conversation as resolved.
Show resolved Hide resolved
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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ observed:
name: vshnpostgres.vshn.appcat.vshn.io-ce52f13
compositionUpdatePolicy: Automatic
parameters:
instances: 1
encryption:
enabled: true
status:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading