Skip to content

Commit 511785f

Browse files
authored
fix: nutanix credentials Secrets owner refs (#711)
**What problem does this PR solve?**: Add `ownerRef` to Nutanix CSI Secret with the corresponding Cluster. CAPX already adds an ownerRef to the PC credentials Secret that CAPX uses so did not add it there. **Which issue(s) this PR fixes**: Fixes https://jira.nutanix.com/browse/NCN-100575 **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent e2ea110 commit 511785f

File tree

4 files changed

+206
-12
lines changed

4 files changed

+206
-12
lines changed

pkg/handlers/generic/lifecycle/ccm/nutanix/handler.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ func (p *provider) Apply(
9898
// However, that would leave the credentials visible in the HelmChartProxy.
9999
// Instead, we'll create the Secret on the remote cluster and reference it in the Helm values.
100100
if clusterConfig.Addons.CCM.Credentials != nil {
101+
err = lifecycleutils.EnsureOwnerRefForSecret(
102+
ctx,
103+
p.client,
104+
clusterConfig.Addons.CCM.Credentials.SecretRef.Name,
105+
cluster,
106+
)
107+
if err != nil {
108+
return fmt.Errorf(
109+
"error updating owner references on Nutanix CCM source Secret: %w",
110+
err,
111+
)
112+
}
101113
key := ctrlclient.ObjectKey{
102114
Name: defaultCredentialsSecretName,
103115
Namespace: defaultHelmReleaseNamespace,

pkg/handlers/generic/lifecycle/csi/nutanix-csi/handler.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,23 @@ func (n *NutanixCSI) Apply(
9797
}
9898

9999
if provider.Credentials != nil {
100+
err := lifecycleutils.EnsureOwnerRefForSecret(
101+
ctx,
102+
n.client,
103+
provider.Credentials.SecretRef.Name,
104+
&req.Cluster,
105+
)
106+
if err != nil {
107+
return fmt.Errorf(
108+
"error updating owner references on Nutanix CSI driver source Secret: %w",
109+
err,
110+
)
111+
}
100112
key := ctrlclient.ObjectKey{
101113
Name: defaultCredentialsSecretName,
102114
Namespace: defaultStorageHelmReleaseNamespace,
103115
}
104-
err := lifecycleutils.CopySecretToRemoteCluster(
116+
err = lifecycleutils.CopySecretToRemoteCluster(
105117
ctx,
106118
n.client,
107119
provider.Credentials.SecretRef.Name,

pkg/handlers/generic/lifecycle/utils/secrets.go

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
1313
"sigs.k8s.io/cluster-api/controllers/remote"
14+
"sigs.k8s.io/cluster-api/util"
1415
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
1516

1617
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/k8s/client"
@@ -25,17 +26,7 @@ func CopySecretToRemoteCluster(
2526
dstSecretKey ctrlclient.ObjectKey,
2627
cluster *clusterv1.Cluster,
2728
) error {
28-
sourceSecret := &corev1.Secret{
29-
TypeMeta: metav1.TypeMeta{
30-
APIVersion: corev1.SchemeGroupVersion.String(),
31-
Kind: "Secret",
32-
},
33-
ObjectMeta: metav1.ObjectMeta{
34-
Name: srcSecretName,
35-
Namespace: cluster.Namespace,
36-
},
37-
}
38-
err := cl.Get(ctx, ctrlclient.ObjectKeyFromObject(sourceSecret), sourceSecret)
29+
sourceSecret, err := getSecretForCluster(ctx, cl, srcSecretName, cluster)
3930
if err != nil {
4031
return err
4132
}
@@ -71,3 +62,51 @@ func CopySecretToRemoteCluster(
7162

7263
return nil
7364
}
65+
66+
// EnsureOwnerRefForSecret will ensure that the secretName Secret has an OwnerReference of the cluster.
67+
func EnsureOwnerRefForSecret(
68+
ctx context.Context,
69+
cl ctrlclient.Client,
70+
secretName string,
71+
cluster *clusterv1.Cluster,
72+
) error {
73+
secret, err := getSecretForCluster(ctx, cl, secretName, cluster)
74+
if err != nil {
75+
return err
76+
}
77+
78+
secret.OwnerReferences = util.EnsureOwnerRef(
79+
secret.OwnerReferences,
80+
metav1.OwnerReference{
81+
APIVersion: clusterv1.GroupVersion.String(),
82+
Kind: cluster.Kind,
83+
UID: cluster.UID,
84+
Name: cluster.Name,
85+
},
86+
)
87+
88+
err = cl.Update(ctx, secret)
89+
if err != nil {
90+
return fmt.Errorf("failed to update Secret with owner references: %w", err)
91+
}
92+
return nil
93+
}
94+
95+
func getSecretForCluster(
96+
ctx context.Context,
97+
c ctrlclient.Client,
98+
secretName string,
99+
cluster *clusterv1.Cluster,
100+
) (*corev1.Secret, error) {
101+
secret := &corev1.Secret{
102+
TypeMeta: metav1.TypeMeta{
103+
APIVersion: corev1.SchemeGroupVersion.String(),
104+
Kind: "Secret",
105+
},
106+
ObjectMeta: metav1.ObjectMeta{
107+
Name: secretName,
108+
Namespace: cluster.Namespace,
109+
},
110+
}
111+
return secret, c.Get(ctx, ctrlclient.ObjectKeyFromObject(secret), secret)
112+
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package utils
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
corev1 "k8s.io/api/core/v1"
13+
"k8s.io/apimachinery/pkg/api/errors"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
17+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
18+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
19+
"sigs.k8s.io/controller-runtime/pkg/client"
20+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
21+
)
22+
23+
var (
24+
testSecret = &corev1.Secret{
25+
ObjectMeta: metav1.ObjectMeta{
26+
Name: "test-secret",
27+
},
28+
}
29+
testSecretWithOwnerRef = &corev1.Secret{
30+
ObjectMeta: metav1.ObjectMeta{
31+
Name: "test-secret",
32+
OwnerReferences: []metav1.OwnerReference{
33+
{
34+
APIVersion: clusterv1.GroupVersion.String(),
35+
Kind: "Cluster",
36+
Name: testCluster.Name,
37+
UID: "12345",
38+
},
39+
},
40+
},
41+
}
42+
43+
testCluster = &clusterv1.Cluster{
44+
TypeMeta: metav1.TypeMeta{
45+
APIVersion: clusterv1.GroupVersion.String(),
46+
Kind: "Cluster",
47+
},
48+
ObjectMeta: metav1.ObjectMeta{
49+
Name: "test-cluster",
50+
},
51+
}
52+
anotherTestCluster = &clusterv1.Cluster{
53+
TypeMeta: metav1.TypeMeta{
54+
APIVersion: clusterv1.GroupVersion.String(),
55+
Kind: "Cluster",
56+
},
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "another-test-cluster",
59+
},
60+
}
61+
)
62+
63+
func Test_EnsureOwnerRefForSecret(t *testing.T) {
64+
t.Parallel()
65+
66+
tests := []struct {
67+
name string
68+
client client.Client
69+
secretName string
70+
cluster *clusterv1.Cluster
71+
wantOwnerRefs int
72+
wantErr error
73+
}{
74+
{
75+
name: "add owner reference to Secret",
76+
client: buildFakeClient(t, testSecret, testCluster),
77+
secretName: testSecret.Name,
78+
cluster: testCluster,
79+
wantOwnerRefs: 1,
80+
},
81+
{
82+
name: "update existing owner reference in Secret",
83+
client: buildFakeClient(t, testSecretWithOwnerRef, testCluster),
84+
secretName: testSecretWithOwnerRef.Name,
85+
cluster: testCluster,
86+
wantOwnerRefs: 1,
87+
},
88+
{
89+
name: "add owner reference to Secret with an existing owner reference",
90+
client: buildFakeClient(t, testSecretWithOwnerRef, anotherTestCluster),
91+
secretName: testSecretWithOwnerRef.Name,
92+
cluster: anotherTestCluster,
93+
wantOwnerRefs: 2,
94+
},
95+
{
96+
name: "should error on a missing Secret",
97+
client: buildFakeClient(t, testSecret, testCluster),
98+
secretName: "missing-secret",
99+
cluster: testCluster,
100+
wantErr: errors.NewNotFound(corev1.Resource("secrets"), "missing-secret"),
101+
},
102+
}
103+
for _, tt := range tests {
104+
t.Run(tt.name, func(t *testing.T) {
105+
t.Parallel()
106+
107+
err := EnsureOwnerRefForSecret(context.Background(), tt.client, tt.secretName, tt.cluster)
108+
require.Equal(t, tt.wantErr, err)
109+
if tt.wantErr != nil {
110+
return
111+
}
112+
// verify that the owner reference was added
113+
secret := &corev1.Secret{}
114+
err = tt.client.Get(
115+
context.Background(),
116+
client.ObjectKey{Namespace: tt.cluster.Namespace, Name: tt.secretName},
117+
secret,
118+
)
119+
assert.NoError(t, err, "failed to get updated secret")
120+
assert.Len(t, secret.OwnerReferences, tt.wantOwnerRefs)
121+
})
122+
}
123+
}
124+
125+
func buildFakeClient(t *testing.T, objs ...client.Object) client.Client {
126+
t.Helper()
127+
clientScheme := runtime.NewScheme()
128+
utilruntime.Must(clientgoscheme.AddToScheme(clientScheme))
129+
utilruntime.Must(clusterv1.AddToScheme(clientScheme))
130+
return fake.NewClientBuilder().WithScheme(clientScheme).WithObjects(objs...).Build()
131+
}

0 commit comments

Comments
 (0)