Skip to content

Commit f680979

Browse files
authored
fix: Make Cluster the owner of image registry credential secret (#648)
**What problem does this PR solve?**: Makes the Cluster resource the owner of image registry credential Secret. CAREN creates this Secret for each Cluster, and it should be deleted when the Cluster is deleted. **Which issue(s) this PR fixes**: Fixes https://jira.nutanix.com/browse/D2IQ-100572 **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. --> All test cases pass. The test is updated to create a Cluster resource, and to provide the handler a client that can read a Cluster resource. **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 8e5cc5b commit f680979

File tree

2 files changed

+67
-16
lines changed

2 files changed

+67
-16
lines changed

pkg/handlers/generic/mutation/imageregistries/credentials/inject.go

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ import (
1212
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1313
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1414
"k8s.io/apimachinery/pkg/runtime"
15+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
1516
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
1617
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
1718
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
1819
ctrl "sigs.k8s.io/controller-runtime"
1920
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
21+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2022

2123
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
2224
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
@@ -69,7 +71,7 @@ func (h *imageRegistriesPatchHandler) Mutate(
6971
vars map[string]apiextensionsv1.JSON,
7072
holderRef runtimehooksv1.HolderReference,
7173
clusterKey ctrlclient.ObjectKey,
72-
_ mutation.ClusterGetter,
74+
clusterGetter mutation.ClusterGetter,
7375
) error {
7476
log := ctrl.LoggerFrom(ctx).WithValues(
7577
"holderRef", holderRef,
@@ -172,7 +174,16 @@ func (h *imageRegistriesPatchHandler) Mutate(
172174
commands...,
173175
)
174176

175-
generateErr = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, clusterKey)
177+
cluster, err := clusterGetter(ctx)
178+
if err != nil {
179+
log.Error(
180+
err,
181+
"failed to get cluster from Image Registry Credentials mutation handler",
182+
)
183+
return err
184+
}
185+
186+
generateErr = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster)
176187
if generateErr != nil {
177188
return generateErr
178189
}
@@ -216,7 +227,16 @@ func (h *imageRegistriesPatchHandler) Mutate(
216227
).Info("adding PreKubeadmCommands to worker node kubeadm config template")
217228
obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, commands...)
218229

219-
generateErr := createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, clusterKey)
230+
cluster, err := clusterGetter(ctx)
231+
if err != nil {
232+
log.Error(
233+
err,
234+
"failed to get cluster from Image Registry Credentials mutation handler",
235+
)
236+
return err
237+
}
238+
239+
generateErr := createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster)
220240
if generateErr != nil {
221241
return generateErr
222242
}
@@ -335,12 +355,12 @@ func createSecretIfNeeded(
335355
ctx context.Context,
336356
c ctrlclient.Client,
337357
registriesWithOptionalCredentials []providerConfig,
338-
clusterKey ctrlclient.ObjectKey,
358+
cluster *clusterv1.Cluster,
339359
) error {
340360
credentialsSecret, err := generateCredentialsSecret(
341361
registriesWithOptionalCredentials,
342-
clusterKey.Name,
343-
clusterKey.Namespace,
362+
cluster.Name,
363+
cluster.Namespace,
344364
)
345365
if err != nil {
346366
return fmt.Errorf(
@@ -352,6 +372,13 @@ func createSecretIfNeeded(
352372
if err := client.ServerSideApply(ctx, c, credentialsSecret, client.ForceOwnership); err != nil {
353373
return fmt.Errorf("failed to apply Image Registry Credentials Secret: %w", err)
354374
}
375+
376+
if err = controllerutil.SetOwnerReference(cluster, credentialsSecret, c.Scheme()); err != nil {
377+
return fmt.Errorf(
378+
"failed to set owner reference on Image Registry Credentials Secret: %w",
379+
err,
380+
)
381+
}
355382
}
356383

357384
return nil

pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import (
1111
"github.com/stretchr/testify/assert"
1212
corev1 "k8s.io/api/core/v1"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/runtime"
15+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1416
"k8s.io/apiserver/pkg/storage/names"
17+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
18+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
1519
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
1620

1721
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
@@ -126,15 +130,13 @@ func TestImageRegistriesPatch(t *testing.T) {
126130
}
127131

128132
var _ = Describe("Generate Image registry patches", func() {
133+
clientScheme := runtime.NewScheme()
134+
utilruntime.Must(clientgoscheme.AddToScheme(clientScheme))
135+
utilruntime.Must(clusterv1.AddToScheme(clientScheme))
136+
129137
patchGenerator := func() mutation.GeneratePatches {
130-
// Always initialize the testEnv variable in the closure.
131-
// This will allow ginkgo to initialize testEnv variable during test execution time.
132-
testEnv := helpers.TestEnv
133-
// use direct client instead of controller client. This will allow the patch handler to read k8s object
134-
// that are written by the tests.
135-
// Test cases writes credentials secret that the mutator handler reads.
136-
// Using direct client will enable reading it immediately.
137-
client, err := testEnv.GetK8sClient()
138+
// Use direct client to allow patch handler to read objects created by tests.
139+
client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
138140
gomega.Expect(err).To(gomega.BeNil())
139141
return mutation.NewMetaGeneratePatchesHandler("", client, NewPatch(client)).(mutation.GeneratePatches)
140142
}
@@ -392,22 +394,44 @@ var _ = Describe("Generate Image registry patches", func() {
392394

393395
// Create credentials secret before each test
394396
BeforeEach(func(ctx SpecContext) {
395-
client, err := helpers.TestEnv.GetK8sClient()
397+
client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
396398
gomega.Expect(err).To(gomega.BeNil())
399+
397400
gomega.Expect(client.Create(
398401
ctx,
399402
newRegistryCredentialsSecret(validSecretName, request.Namespace),
400403
)).To(gomega.BeNil())
404+
405+
gomega.Expect(client.Create(
406+
ctx,
407+
&clusterv1.Cluster{
408+
ObjectMeta: metav1.ObjectMeta{
409+
Name: request.ClusterName,
410+
Namespace: request.Namespace,
411+
},
412+
},
413+
)).To(gomega.BeNil())
401414
})
402415

403416
// Delete credentials secret after each test
404417
AfterEach(func(ctx SpecContext) {
405-
client, err := helpers.TestEnv.GetK8sClient()
418+
client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
406419
gomega.Expect(err).To(gomega.BeNil())
420+
407421
gomega.Expect(client.Delete(
408422
ctx,
409423
newRegistryCredentialsSecret(validSecretName, request.Namespace),
410424
)).To(gomega.BeNil())
425+
426+
gomega.Expect(client.Delete(
427+
ctx,
428+
&clusterv1.Cluster{
429+
ObjectMeta: metav1.ObjectMeta{
430+
Name: request.ClusterName,
431+
Namespace: request.Namespace,
432+
},
433+
},
434+
)).To(gomega.BeNil())
411435
})
412436
// create test node for each case
413437
for testIdx := range testDefs {

0 commit comments

Comments
 (0)