From 6c127d7aa8cd5ab6c1bc4df0117705feaac458d9 Mon Sep 17 00:00:00 2001 From: "Kistner, Dominic" Date: Wed, 18 May 2022 08:42:16 +0200 Subject: [PATCH] Adapt worker and controlplane controller to use managed app credentials --- pkg/controller/controlplane/valuesprovider.go | 16 ++++-- .../controlplane/valuesprovider_test.go | 17 +++++++ pkg/controller/worker/actuator.go | 20 +++++++- .../worker/machine_dependencies_test.go | 17 ++++++- pkg/controller/worker/machines.go | 4 +- pkg/controller/worker/machines_test.go | 51 ++++++++++--------- 6 files changed, 93 insertions(+), 32 deletions(-) diff --git a/pkg/controller/controlplane/valuesprovider.go b/pkg/controller/controlplane/valuesprovider.go index d2ce39d22..1c3a9bad5 100644 --- a/pkg/controller/controlplane/valuesprovider.go +++ b/pkg/controller/controlplane/valuesprovider.go @@ -22,6 +22,7 @@ import ( api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper" + "github.com/gardener/gardener-extension-provider-openstack/pkg/internal/managedappcredential" "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/utils" @@ -277,10 +278,19 @@ func (vp *valuesProvider) GetConfigChartValues( return nil, err } - // Get credentials - credentials, err := openstack.GetCredentials(ctx, vp.Client(), cp.Spec.SecretRef, false) + // Try to read the credentials of the managed application credential if exists. + credentials, _, err := managedappcredential.GetCredentials(ctx, vp.Client(), cp.Namespace) if err != nil { - return nil, fmt.Errorf("could not get service account from secret '%s/%s': %w", cp.Spec.SecretRef.Namespace, cp.Spec.SecretRef.Name, err) + return nil, err + } + + if credentials == nil { + // If no managed application credential exists take the regular user. + userCredentials, err := openstack.GetCredentials(ctx, vp.Client(), cp.Spec.SecretRef, false) + if err != nil { + return nil, fmt.Errorf("could not get service account from secret '%s/%s': %w", cp.Spec.SecretRef.Namespace, cp.Spec.SecretRef.Name, err) + } + credentials = userCredentials } return getConfigChartValues(cpConfig, infraStatus, cloudProfileConfig, cp, credentials, cluster) diff --git a/pkg/controller/controlplane/valuesprovider_test.go b/pkg/controller/controlplane/valuesprovider_test.go index 389a1c4e4..6c27ebd23 100644 --- a/pkg/controller/controlplane/valuesprovider_test.go +++ b/pkg/controller/controlplane/valuesprovider_test.go @@ -35,6 +35,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/pointer" @@ -299,6 +300,7 @@ var _ = Describe("ValuesProvider", func() { } It("should return correct config chart values", func() { + expectGetManagedApplicationCredentialSecretToFail(ctx, c) c.EXPECT().Get(ctx, cpSecretKey, &corev1.Secret{}).DoAndReturn(clientGet(cpSecret)) values, err := vp.GetConfigChartValues(ctx, cp, clusterK8sLessThan119) @@ -307,6 +309,7 @@ var _ = Describe("ValuesProvider", func() { }) It("should return correct config chart values with load balancer classes", func() { + expectGetManagedApplicationCredentialSecretToFail(ctx, c) c.EXPECT().Get(ctx, cpSecretKey, &corev1.Secret{}).DoAndReturn(clientGet(cpSecret)) var ( @@ -398,6 +401,7 @@ var _ = Describe("ValuesProvider", func() { }) It("should return correct config chart values with load balancer classes with purpose", func() { + expectGetManagedApplicationCredentialSecretToFail(ctx, c) c.EXPECT().Get(ctx, cpSecretKey, &corev1.Secret{}).DoAndReturn(clientGet(cpSecret)) var ( @@ -455,6 +459,7 @@ var _ = Describe("ValuesProvider", func() { "applicationCredentialSecret": []byte(`app-secret`), } + expectGetManagedApplicationCredentialSecretToFail(ctx, c) c.EXPECT().Get(ctx, cpSecretKey, &corev1.Secret{}).DoAndReturn(clientGet(&secret2)) expectedValues := utils.MergeMaps(configChartValues, map[string]interface{}{ @@ -647,3 +652,15 @@ func clientGet(result runtime.Object) interface{} { return nil } } + +func expectGetManagedApplicationCredentialSecretToFail(ctx context.Context, c *mockclient.MockClient) { + c.EXPECT().Get( + ctx, + client.ObjectKey{Namespace: namespace, Name: "cloudprovider-application-credential"}, + gomock.AssignableToTypeOf(&corev1.Secret{}), + ).Return(&apierrors.StatusError{ + ErrStatus: metav1.Status{ + Reason: metav1.StatusReasonNotFound, + }, + }) +} diff --git a/pkg/controller/worker/actuator.go b/pkg/controller/worker/actuator.go index a86ad0ae6..012e8d769 100644 --- a/pkg/controller/worker/actuator.go +++ b/pkg/controller/worker/actuator.go @@ -21,6 +21,7 @@ import ( api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper" "github.com/gardener/gardener-extension-provider-openstack/pkg/imagevector" + "github.com/gardener/gardener-extension-provider-openstack/pkg/internal/managedappcredential" "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack/client" @@ -32,6 +33,7 @@ import ( extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" gardener "github.com/gardener/gardener/pkg/client/kubernetes" "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -85,7 +87,17 @@ func (d *delegateFactory) WorkerDelegate(ctx context.Context, worker *extensions return nil, err } - openstackClient, err := client.NewOpenStackClientFromSecretRef(ctx, d.Client(), worker.Spec.SecretRef, &keyStoneURL) + secretRef := &worker.Spec.SecretRef + _, appCredentialSecretRef, err := managedappcredential.GetCredentials(ctx, d.Client(), worker.Namespace) + if err != nil { + return nil, err + } + + if appCredentialSecretRef != nil { + secretRef = appCredentialSecretRef + } + + openstackClient, err := client.NewOpenStackClientFromSecretRef(ctx, d.Client(), *secretRef, &keyStoneURL) if err != nil { return nil, fmt.Errorf("failed to create openstack client: %w", err) } @@ -99,6 +111,7 @@ func (d *delegateFactory) WorkerDelegate(ctx context.Context, worker *extensions worker, cluster, openstackClient, + secretRef, ) } @@ -116,7 +129,8 @@ type workerDelegate struct { machineDeployments worker.MachineDeployments machineImages []api.MachineImage - openstackClient client.Factory + openstackClient client.Factory + openstackSecretRef *corev1.SecretReference } // NewWorkerDelegate creates a new context for a worker reconciliation. @@ -129,6 +143,7 @@ func NewWorkerDelegate( worker *extensionsv1alpha1.Worker, cluster *extensionscontroller.Cluster, openstackClient client.Factory, + openstackSecretRef *corev1.SecretReference, ) (genericactuator.WorkerDelegate, error) { config, err := helper.CloudProfileConfigFromCluster(cluster) if err != nil { @@ -145,5 +160,6 @@ func NewWorkerDelegate( cluster: cluster, worker: worker, openstackClient: openstackClient, + openstackSecretRef: openstackSecretRef, }, nil } diff --git a/pkg/controller/worker/machine_dependencies_test.go b/pkg/controller/worker/machine_dependencies_test.go index be57ad704..258dcea47 100644 --- a/pkg/controller/worker/machine_dependencies_test.go +++ b/pkg/controller/worker/machine_dependencies_test.go @@ -39,6 +39,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -83,7 +84,9 @@ var _ = Describe("#MachineDependencies", func() { var ( clusterName = "shoot--foobar--openstack" namespace = clusterName - w *extensionsv1alpha1.Worker + + w *extensionsv1alpha1.Worker + openstackSecretRef *corev1.SecretReference ) BeforeEach(func() { @@ -92,6 +95,12 @@ var _ = Describe("#MachineDependencies", func() { Namespace: namespace, }, } + + openstackSecretRef = &corev1.SecretReference{ + Name: "secret", + Namespace: namespace, + } + osFactory.EXPECT().Compute().AnyTimes().Return(computeClient, nil) }) @@ -103,6 +112,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) ctx := context.Background() @@ -137,6 +147,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) computeClient.EXPECT().CreateServerGroup(prefixMatch(serverGroupPrefix(clusterName, pool1)), policy).Return(&servergroups.ServerGroup{ @@ -182,6 +193,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) computeClient.EXPECT().CreateServerGroup(prefixMatch(serverGroupPrefix(clusterName, poolName)), policy).Return(&servergroups.ServerGroup{ @@ -251,6 +263,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) computeClient.EXPECT().ListServerGroups().Return([]servergroups.ServerGroup{ @@ -305,6 +318,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) computeClient.EXPECT().ListServerGroups().Return([]servergroups.ServerGroup{ @@ -367,6 +381,7 @@ var _ = Describe("#MachineDependencies", func() { w, newClusterWithDefaultCloudProfileConfig(clusterName), osFactory, + openstackSecretRef, ) computeClient.EXPECT().ListServerGroups().Return([]servergroups.ServerGroup{ diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 1fc533909..6b2bd0c60 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -148,8 +148,8 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { "kubernetes.io-role-node": "1", }, "credentialsSecretRef": map[string]interface{}{ - "name": w.worker.Spec.SecretRef.Name, - "namespace": w.worker.Spec.SecretRef.Namespace, + "name": w.openstackSecretRef.Name, + "namespace": w.openstackSecretRef.Namespace, }, "secret": map[string]interface{}{ "cloudConfig": string(pool.UserData), diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index cf5f60d17..6e5aa44f6 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -71,7 +71,7 @@ var _ = Describe("Machines", func() { }) Context("workerDelegate", func() { - workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(nil, nil, nil), nil, "", nil, nil, nil) + workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(nil, nil, nil), nil, "", nil, nil, nil, nil) Describe("#MachineClassKind", func() { It("should return the correct kind of the machine class", func() { @@ -134,6 +134,7 @@ var _ = Describe("Machines", func() { shootVersion string scheme *runtime.Scheme decoder runtime.Decoder + openstackSecretRef *corev1.SecretReference cloudProfileConfig *api.CloudProfileConfig cloudProfileConfigJSON []byte clusterWithoutImages *extensionscontroller.Cluster @@ -271,10 +272,6 @@ var _ = Describe("Machines", func() { Namespace: namespace, }, Spec: extensionsv1alpha1.WorkerSpec{ - SecretRef: corev1.SecretReference{ - Name: "secret", - Namespace: namespace, - }, Region: region, InfrastructureProviderStatus: &runtime.RawExtension{ Raw: encode(&api.InfrastructureStatus{ @@ -351,7 +348,12 @@ var _ = Describe("Machines", func() { workerPoolHash1, _ = worker.WorkerPoolHash(w.Spec.Pools[0], cluster) workerPoolHash2, _ = worker.WorkerPoolHash(w.Spec.Pools[1], cluster) - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, clusterWithoutImages, nil) + openstackSecretRef = &corev1.SecretReference{ + Name: "secret", + Namespace: namespace, + } + + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, clusterWithoutImages, nil, openstackSecretRef) }) Describe("machine images", func() { @@ -427,10 +429,10 @@ var _ = Describe("Machines", func() { machineClassWithHashPool2Zone2 = fmt.Sprintf("%s-%s", machineClassNamePool2Zone2, workerPoolHash2) ) - addNameAndSecretToMachineClass(machineClassPool1Zone1, machineClassWithHashPool1Zone1, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool1Zone2, machineClassWithHashPool1Zone2, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool2Zone1, machineClassWithHashPool2Zone1, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool2Zone2, machineClassWithHashPool2Zone2, w.Spec.SecretRef) + addNameAndSecretToMachineClass(machineClassPool1Zone1, machineClassWithHashPool1Zone1, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool1Zone2, machineClassWithHashPool1Zone2, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool2Zone1, machineClassWithHashPool2Zone1, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool2Zone2, machineClassWithHashPool2Zone2, openstackSecretRef) addNodeTemplateToMachineClass(machineClassPool1Zone1, newNodeTemplateZone1) addNodeTemplateToMachineClass(machineClassPool1Zone2, newNodeTemplateZone2) @@ -490,7 +492,7 @@ var _ = Describe("Machines", func() { It("should return the expected machine deployments for profile image types", func() { setup(region, machineImage, "") - workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil) + workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil, openstackSecretRef) // Test workerDelegate.DeployMachineClasses() chartApplier. @@ -544,7 +546,7 @@ var _ = Describe("Machines", func() { It("should return the expected machine deployments for profile image types with id", func() { setup(regionWithImages, "", machineImageID) - workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithRegion, clusterWithRegion, nil) + workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithRegion, clusterWithRegion, nil, openstackSecretRef) clusterWithRegion.Shoot.Spec.Hibernation = &gardencorev1beta1.Hibernation{Enabled: pointer.BoolPtr(true)} // Test workerDelegate.DeployMachineClasses() @@ -652,7 +654,7 @@ var _ = Describe("Machines", func() { }, } - workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithServerGroup, cluster, nil) + workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithServerGroup, cluster, nil, openstackSecretRef) // Test workerDelegate.DeployMachineClasses() workerPoolHash1, _ := worker.WorkerPoolHash(w.Spec.Pools[0], cluster, serverGroupID1) @@ -681,14 +683,15 @@ var _ = Describe("Machines", func() { machineClassWithHashPool1Zone2 := fmt.Sprintf("%s-%s", machineClassNamePool1Zone2, workerPoolHash1) machineClassWithHashPool2Zone1 := fmt.Sprintf("%s-%s", machineClassNamePool2Zone1, workerPoolHash2) machineClassWithHashPool2Zone2 := fmt.Sprintf("%s-%s", machineClassNamePool2Zone2, workerPoolHash2) - addNameAndSecretToMachineClass(machineClassPool1Zone1, machineClassWithHashPool1Zone1, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool1Zone2, machineClassWithHashPool1Zone2, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool2Zone1, machineClassWithHashPool2Zone1, w.Spec.SecretRef) - addNameAndSecretToMachineClass(machineClassPool2Zone2, machineClassWithHashPool2Zone2, w.Spec.SecretRef) + addNameAndSecretToMachineClass(machineClassPool1Zone1, machineClassWithHashPool1Zone1, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool1Zone2, machineClassWithHashPool1Zone2, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool2Zone1, machineClassWithHashPool2Zone1, openstackSecretRef) + addNameAndSecretToMachineClass(machineClassPool2Zone2, machineClassWithHashPool2Zone2, openstackSecretRef) addNodeTemplateToMachineClass(machineClassPool1Zone1, nodeTemplateZone1) addNodeTemplateToMachineClass(machineClassPool1Zone2, nodeTemplateZone2) addNodeTemplateToMachineClass(machineClassPool2Zone1, nodeTemplateZone1) addNodeTemplateToMachineClass(machineClassPool2Zone2, nodeTemplateZone2) + machineClasses := map[string]interface{}{"machineClasses": []map[string]interface{}{ machineClassPool1Zone1, machineClassPool1Zone2, @@ -728,7 +731,7 @@ var _ = Describe("Machines", func() { }, } - workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithServerGroup, cluster, nil) + workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithServerGroup, cluster, nil, openstackSecretRef) err := workerDelegate.DeployMachineClasses(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal(`server group is required for pool "pool-1", but no server group dependency found`)) @@ -737,7 +740,7 @@ var _ = Describe("Machines", func() { It("should fail because the version is invalid", func() { clusterWithoutImages.Shoot.Spec.Kubernetes.Version = "invalid" - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil) + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil, openstackSecretRef) result, err := workerDelegate.GenerateMachineDeployments(context.TODO()) Expect(err).To(HaveOccurred()) @@ -747,7 +750,7 @@ var _ = Describe("Machines", func() { It("should fail because the infrastructure status cannot be decoded", func() { w.Spec.InfrastructureProviderStatus = &runtime.RawExtension{} - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil) + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil, openstackSecretRef) result, err := workerDelegate.GenerateMachineDeployments(context.TODO()) Expect(err).To(HaveOccurred()) @@ -759,7 +762,7 @@ var _ = Describe("Machines", func() { Raw: encode(&api.InfrastructureStatus{}), } - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil) + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil, openstackSecretRef) result, err := workerDelegate.GenerateMachineDeployments(context.TODO()) Expect(err).To(HaveOccurred()) @@ -769,7 +772,7 @@ var _ = Describe("Machines", func() { It("should fail because the machine image for this cloud profile cannot be found", func() { clusterWithoutImages.CloudProfile.Name = "another-cloud-profile" - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, clusterWithoutImages, nil) + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, clusterWithoutImages, nil, openstackSecretRef) result, err := workerDelegate.GenerateMachineDeployments(context.TODO()) Expect(err).To(HaveOccurred()) @@ -790,7 +793,7 @@ var _ = Describe("Machines", func() { NodeConditions: testNodeConditions, } - workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil) + workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster, nil, openstackSecretRef) result, err := workerDelegate.GenerateMachineDeployments(context.TODO()) resultSettings := result[0].MachineConfiguration @@ -841,7 +844,7 @@ func addNodeTemplateToMachineClass(class map[string]interface{}, nodeTemplate ma class["nodeTemplate"] = nodeTemplate } -func addNameAndSecretToMachineClass(class map[string]interface{}, name string, credentialsSecretRef corev1.SecretReference) { +func addNameAndSecretToMachineClass(class map[string]interface{}, name string, credentialsSecretRef *corev1.SecretReference) { class["name"] = name class["labels"] = map[string]string{ v1beta1constants.GardenerPurpose: genericworkeractuator.GardenPurposeMachineClass,