From 88b088f97833f410f8e927daff8039ea32cbcd62 Mon Sep 17 00:00:00 2001 From: Tanvir Tatla Date: Fri, 1 Dec 2023 09:25:58 -0800 Subject: [PATCH 1/5] Fix registry mirror issues in packages --- Makefile | 2 +- config/default/kustomization.yaml | 1 - config/manifest/eksa-components.yaml | 43 +++++++++++-- config/prod/kustomization.yaml | 1 + config/rbac/role.yaml | 20 +++++- config/rbac/role_binding.yaml | 17 ++++- config/rbac_default/apply-namespace.yaml | 11 ++++ config/rbac_default/kustomization.yaml | 38 +++++++++++ controllers/cluster_controller.go | 10 ++- controllers/mocks/cluster_controller.go | 14 +++++ pkg/api/v1alpha1/nodeupgrade_types.go | 1 + pkg/clustermanager/cluster_manager.go | 4 ++ .../packagecontrollerclient.go | 63 +++++++++++++++++++ pkg/workflows/create.go | 6 ++ pkg/workflows/interfaces/interfaces.go | 1 + pkg/workflows/interfaces/mocks/clients.go | 14 +++++ 16 files changed, 234 insertions(+), 12 deletions(-) create mode 100644 config/rbac_default/apply-namespace.yaml create mode 100644 config/rbac_default/kustomization.yaml diff --git a/Makefile b/Makefile index 2ac4807006d5..12cdb50fdba1 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,7 @@ OUTPUT_DIR := _output OUTPUT_BIN_DIR := ${OUTPUT_DIR}/bin KUSTOMIZE := $(TOOLS_BIN_DIR)/kustomize -KUSTOMIZE_VERSION := 4.2.0 +KUSTOMIZE_VERSION := 4.5.6 KUSTOMIZE_OUTPUT_BIN_DIR="${OUTPUT_DIR}/kustomize-bin/" diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index a0bc77d839b7..5462cb0bfb9e 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -17,7 +17,6 @@ resources: bases: - ../crd -- ../rbac - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml diff --git a/config/manifest/eksa-components.yaml b/config/manifest/eksa-components.yaml index c763a96ecc04..14effff0286b 100644 --- a/config/manifest/eksa-components.yaml +++ b/config/manifest/eksa-components.yaml @@ -6633,6 +6633,28 @@ metadata: --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role +metadata: + creationTimestamp: null + name: eksa-packages-role + namespace: eksa-packages +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - patch + - update +- apiGroups: + - packages.eks.amazonaws.com + resources: + - packagebundlecontrollers + verbs: + - delete +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role metadata: name: eksa-leader-election-role namespace: eksa-system @@ -6683,12 +6705,6 @@ rules: verbs: - patch - update -- apiGroups: - - packages.eks.amazonaws.com - resources: - - packagebundlecontrollers - verbs: - - delete --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -6711,6 +6727,7 @@ rules: verbs: - create - delete + - get - apiGroups: - "" resources: @@ -7062,6 +7079,20 @@ rules: --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding +metadata: + name: eksa-packages-rolebinding + namespace: eksa-packages +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: eksa-packages-role +subjects: +- kind: ServiceAccount + name: eksa-controller-manager + namespace: eksa-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding metadata: name: eksa-leader-election-rolebinding namespace: eksa-system diff --git a/config/prod/kustomization.yaml b/config/prod/kustomization.yaml index 2f083a064837..f62546cedf09 100644 --- a/config/prod/kustomization.yaml +++ b/config/prod/kustomization.yaml @@ -2,6 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization resources: - ../default +- ../rbac_default images: - name: controller diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b7b7a278f06f..a6fd83ad0005 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -21,6 +21,7 @@ rules: verbs: - create - delete + - get - apiGroups: - "" resources: @@ -376,13 +377,14 @@ kind: Role metadata: creationTimestamp: null name: manager-role - namespace: eksa-system + namespace: eksa-packages rules: - apiGroups: - "" resources: - secrets verbs: + - get - patch - update - apiGroups: @@ -391,3 +393,19 @@ rules: - packagebundlecontrollers verbs: - delete + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + creationTimestamp: null + name: manager-role + namespace: eksa-system +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - patch + - update diff --git a/config/rbac/role_binding.yaml b/config/rbac/role_binding.yaml index 293aab94c6ef..e73f2da45556 100644 --- a/config/rbac/role_binding.yaml +++ b/config/rbac/role_binding.yaml @@ -24,4 +24,19 @@ roleRef: subjects: - kind: ServiceAccount name: controller-manager - namespace: system \ No newline at end of file + namespace: system + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: manager-rolebinding + namespace: eksa-packages +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: manager-role +subjects: +- kind: ServiceAccount + name: controller-manager + namespace: system diff --git a/config/rbac_default/apply-namespace.yaml b/config/rbac_default/apply-namespace.yaml new file mode 100644 index 000000000000..a2447505bec2 --- /dev/null +++ b/config/rbac_default/apply-namespace.yaml @@ -0,0 +1,11 @@ +namePrefix: +- path: metadata/namespace + kind: ServiceAccount +- path: subjects/name + kind: RoleBinding +- path: subjects/namespace + kind: RoleBinding +- path: subjects/name + kind: ClusterRoleBinding +- path: subjects/namespace + kind: ClusterRoleBinding \ No newline at end of file diff --git a/config/rbac_default/kustomization.yaml b/config/rbac_default/kustomization.yaml new file mode 100644 index 000000000000..0c7a0541679a --- /dev/null +++ b/config/rbac_default/kustomization.yaml @@ -0,0 +1,38 @@ +namePrefix: eksa- + +bases: +- ../rbac + +configurations: +- ./apply-namespace.yaml + +transformers: + - |- + apiVersion: builtin + kind: NamespaceTransformer + metadata: + name: notImportantHere2 + namespace: eksa-system + unsetOnly: true + +patchesJson6902: +- patch: |- + - op: replace + path: /metadata/name + value: eksa-packages-rolebinding + target: + group: rbac.authorization.k8s.io + kind: RoleBinding + name: eksa-manager-rolebinding + namespace: eksa-packages + version: v1 +- patch: |- + - op: replace + path: /metadata/name + value: eksa-packages-role + target: + group: rbac.authorization.k8s.io + kind: Role + name: eksa-manager-role + namespace: eksa-packages + version: v1 \ No newline at end of file diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go index c99ebc47012b..e65de80b3f5f 100644 --- a/controllers/cluster_controller.go +++ b/controllers/cluster_controller.go @@ -58,6 +58,7 @@ type PackagesClient interface { EnableFullLifecycle(ctx context.Context, log logr.Logger, clusterName, kubeConfig string, chart *v1alpha1.Image, registry *registrymirror.RegistryMirror, options ...curatedpackages.PackageControllerClientOpt) error ReconcileDelete(context.Context, logr.Logger, curatedpackages.KubeDeleter, *anywherev1.Cluster) error Reconcile(context.Context, logr.Logger, client.Client, *anywherev1.Cluster) error + UpdateSecrets(ctx context.Context, client client.Client, cluster *anywherev1.Cluster) error } type ProviderClusterReconcilerRegistry interface { @@ -174,7 +175,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager, log logr.Logger) // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch;update // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;delete // +kubebuilder:rbac:groups="",namespace=eksa-system,resources=secrets,verbs=patch;update -// +kubebuilder:rbac:groups="",resources=namespaces,verbs=create;delete +// +kubebuilder:rbac:groups="",resources=namespaces,verbs=create;delete;get // +kubebuilder:rbac:groups="",resources=nodes,verbs=list // +kubebuilder:rbac:groups=addons.cluster.x-k8s.io,resources=clusterresourcesets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=anywhere.eks.amazonaws.com,resources=clusters;gitopsconfigs;snowmachineconfigs;snowdatacenterconfigs;snowippools;vspheredatacenterconfigs;vspheremachineconfigs;dockerdatacenterconfigs;tinkerbellmachineconfigs;tinkerbelldatacenterconfigs;cloudstackdatacenterconfigs;cloudstackmachineconfigs;nutanixdatacenterconfigs;nutanixmachineconfigs;awsiamconfigs;oidcconfigs;awsiamconfigs;fluxconfigs,verbs=get;list;watch;update;patch @@ -194,7 +195,8 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager, log logr.Logger) // +kubebuilder:rbac:groups=bmc.tinkerbell.org,resources=machines,verbs=list;watch // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awssnowclusters;awssnowmachinetemplates;awssnowippools;vsphereclusters;vspheremachinetemplates;dockerclusters;dockermachinetemplates;tinkerbellclusters;tinkerbellmachinetemplates;cloudstackclusters;cloudstackmachinetemplates;nutanixclusters;nutanixmachinetemplates,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=packages.eks.amazonaws.com,resources=packages,verbs=create;delete;get;list;patch;update;watch -// +kubebuilder:rbac:groups=packages.eks.amazonaws.com,namespace=eksa-system,resources=packagebundlecontrollers,verbs=delete +// +kubebuilder:rbac:groups=packages.eks.amazonaws.com,namespace=eksa-packages,resources=packagebundlecontrollers,verbs=delete +// +kubebuilder:rbac:groups="",namespace=eksa-packages,resources=secrets,verbs=get;patch;update // +kubebuilder:rbac:groups=anywhere.eks.amazonaws.com,resources=eksareleases,verbs=get;list;watch // The eksareleases permissions are being moved to the ClusterRole due to client trying to list this resource from cache. // When trying to list resources not already in cache, it starts an informer for that type using the scope of the cache. @@ -386,6 +388,10 @@ func (r *ClusterReconciler) preClusterProviderReconcile(ctx context.Context, log } } + if err := r.packagesClient.UpdateSecrets(ctx, r.client, cluster); err != nil { + return controller.Result{}, err + } + return controller.Result{}, nil } diff --git a/controllers/mocks/cluster_controller.go b/controllers/mocks/cluster_controller.go index aef7a66e247e..e98c0b5febc3 100644 --- a/controllers/mocks/cluster_controller.go +++ b/controllers/mocks/cluster_controller.go @@ -89,6 +89,20 @@ func (mr *MockPackagesClientMockRecorder) ReconcileDelete(arg0, arg1, arg2, arg3 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileDelete", reflect.TypeOf((*MockPackagesClient)(nil).ReconcileDelete), arg0, arg1, arg2, arg3) } +// UpdateSecrets mocks base method. +func (m *MockPackagesClient) UpdateSecrets(ctx context.Context, client client.Client, cluster *v1alpha1.Cluster) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateSecrets", ctx, client, cluster) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateSecrets indicates an expected call of UpdateSecrets. +func (mr *MockPackagesClientMockRecorder) UpdateSecrets(ctx, client, cluster interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSecrets", reflect.TypeOf((*MockPackagesClient)(nil).UpdateSecrets), ctx, client, cluster) +} + // MockProviderClusterReconcilerRegistry is a mock of ProviderClusterReconcilerRegistry interface. type MockProviderClusterReconcilerRegistry struct { ctrl *gomock.Controller diff --git a/pkg/api/v1alpha1/nodeupgrade_types.go b/pkg/api/v1alpha1/nodeupgrade_types.go index e9a48fb528b8..561846e7418e 100644 --- a/pkg/api/v1alpha1/nodeupgrade_types.go +++ b/pkg/api/v1alpha1/nodeupgrade_types.go @@ -38,6 +38,7 @@ type NodeUpgradeSpec struct { Machine corev1.ObjectReference `json:"machine"` // TODO(in-place): Determine if there's a way to get these dynamically instead of expecting it from the CRD. + KubernetesVersion string `json:"kubernetesVersion"` EtcdVersion *string `json:"etcdVersion,omitempty"` CoreDNSVersion *string `json:"coreDNSVersion,omitempty"` diff --git a/pkg/clustermanager/cluster_manager.go b/pkg/clustermanager/cluster_manager.go index 3da0545a6de0..a22d7f0a8745 100644 --- a/pkg/clustermanager/cluster_manager.go +++ b/pkg/clustermanager/cluster_manager.go @@ -1137,6 +1137,10 @@ func (c *ClusterManager) CreateEKSANamespace(ctx context.Context, cluster *types return c.clusterClient.CreateNamespaceIfNotPresent(ctx, cluster.KubeconfigFile, constants.EksaSystemNamespace) } +func (c *ClusterManager) CreatePackagesNamespace(ctx context.Context, cluster *types.Cluster) error { + return c.clusterClient.CreateNamespaceIfNotPresent(ctx, cluster.KubeconfigFile, constants.EksaPackagesName) +} + // CreateEKSAResources applies the eks-a cluster specs (cluster, datacenterconfig, machine configs, etc.), as well as the // release bundle to the cluster. Before applying the spec, we pause eksa controller cluster and datacenter webhook validation // so that the cluster spec can be created or updated in the cluster without webhook validation error. diff --git a/pkg/curatedpackages/packagecontrollerclient.go b/pkg/curatedpackages/packagecontrollerclient.go index f2b5495d98e4..41975279be17 100644 --- a/pkg/curatedpackages/packagecontrollerclient.go +++ b/pkg/curatedpackages/packagecontrollerclient.go @@ -4,6 +4,7 @@ import ( "context" _ "embed" "encoding/base64" + "encoding/json" "fmt" "strconv" "strings" @@ -38,6 +39,17 @@ const ( valueFileName = "values.yaml" ) +type dockerConfig struct { + Auths map[string]*dockerAuth `json:"auths"` +} + +type dockerAuth struct { + Username string `json:"username"` + Password string `json:"password"` + Email string `json:"email"` + Auth string `json:"auth"` +} + type PackageControllerClientOpt func(client *PackageControllerClient) type PackageControllerClient struct { @@ -75,6 +87,7 @@ type PackageControllerClient struct { // registryAccessTester test if the aws credential has access to registry registryAccessTester RegistryAccessTester + flc bool } // ClientBuilder returns a k8s client for the specified cluster. @@ -236,6 +249,56 @@ func (pc *PackageControllerClient) Enable(ctx context.Context) error { return nil } +func (pc *PackageControllerClient) UpdateSecrets(ctx context.Context, client client.Client, cluster *anywherev1.Cluster) error { + secretKey := types.NamespacedName{ + Namespace: constants.EksaPackagesName, + Name: "registry-mirror-cred", + } + secret := &corev1.Secret{} + credErr := client.Get(ctx, secretKey, secret) + err := fillRegistrySecret(cluster.Name, cluster.Spec.RegistryMirrorConfiguration, secret) + if err != nil { + return err + } + + if apierrors.IsNotFound(credErr) { + return client.Create(ctx, secret) + } else if credErr == nil { + return client.Update(ctx, secret) + } + return credErr +} + +func fillRegistrySecret(clusterName string, registry *anywherev1.RegistryMirrorConfiguration, secret *corev1.Secret) error { + caDataName := clusterName + "_ca.crt" + insecureDataName := clusterName + "_insecure" + secret.Data[caDataName] = []byte(registry.CACertContent) + secret.Data[insecureDataName] = []byte(strconv.FormatBool(registry.InsecureSkipVerify)) + + dconfig := &dockerConfig{Auths: make(map[string]*dockerAuth)} + err := json.Unmarshal(secret.Data["config.json"], dconfig) + if err != nil { + return err + } + username, password, err := config.ReadCredentials() + if err != nil { + return err + } + dconfig.Auths[registry.Endpoint] = &dockerAuth{ + Username: username, + Password: password, + Email: "test@test.com", + Auth: base64.StdEncoding.EncodeToString([]byte(username + ":" + password)), + } + + configJson, err := json.Marshal(dconfig) + if err != nil { + return err + } + secret.Data["config.json"] = configJson + return nil +} + // GetCuratedPackagesRegistries gets value for configurable registries from PBC. func (pc *PackageControllerClient) GetCuratedPackagesRegistries(ctx context.Context) (sourceRegistry, defaultRegistry, defaultImageRegistry string) { sourceRegistry = publicProdECR diff --git a/pkg/workflows/create.go b/pkg/workflows/create.go index d4ef7f36735c..6983b242cf24 100644 --- a/pkg/workflows/create.go +++ b/pkg/workflows/create.go @@ -252,6 +252,12 @@ func (s *CreateWorkloadClusterTask) Run(ctx context.Context, commandContext *tas return &CollectDiagnosticsTask{} } + err = commandContext.ClusterManager.CreatePackagesNamespace(ctx, workloadCluster) + if err != nil { + commandContext.SetError(err) + return &CollectDiagnosticsTask{} + } + logger.Info("Installing cluster-api providers on workload cluster") err = commandContext.ClusterManager.InstallCAPI(ctx, commandContext.ClusterSpec, commandContext.WorkloadCluster, commandContext.Provider) if err != nil { diff --git a/pkg/workflows/interfaces/interfaces.go b/pkg/workflows/interfaces/interfaces.go index fe0343fef74c..79d6acfa5fc2 100644 --- a/pkg/workflows/interfaces/interfaces.go +++ b/pkg/workflows/interfaces/interfaces.go @@ -33,6 +33,7 @@ type ClusterManager interface { SaveLogsWorkloadCluster(ctx context.Context, provider providers.Provider, spec *cluster.Spec, cluster *types.Cluster) error InstallCustomComponents(ctx context.Context, clusterSpec *cluster.Spec, cluster *types.Cluster, provider providers.Provider) error CreateEKSANamespace(ctx context.Context, cluster *types.Cluster) error + CreatePackagesNamespace(ctx context.Context, cluster *types.Cluster) error CreateEKSAResources(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec, datacenterConfig providers.DatacenterConfig, machineConfigs []providers.MachineConfig) error ApplyBundles(ctx context.Context, clusterSpec *cluster.Spec, cluster *types.Cluster) error ApplyReleases(ctx context.Context, clusterSpec *cluster.Spec, cluster *types.Cluster) error diff --git a/pkg/workflows/interfaces/mocks/clients.go b/pkg/workflows/interfaces/mocks/clients.go index 81f3f0b291b1..5950cb4b5702 100644 --- a/pkg/workflows/interfaces/mocks/clients.go +++ b/pkg/workflows/interfaces/mocks/clients.go @@ -195,6 +195,20 @@ func (mr *MockClusterManagerMockRecorder) CreateEKSAResources(arg0, arg1, arg2, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateEKSAResources", reflect.TypeOf((*MockClusterManager)(nil).CreateEKSAResources), arg0, arg1, arg2, arg3, arg4) } +// CreatePackagesNamespace mocks base method. +func (m *MockClusterManager) CreatePackagesNamespace(arg0 context.Context, arg1 *types.Cluster) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreatePackagesNamespace", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreatePackagesNamespace indicates an expected call of CreatePackagesNamespace. +func (mr *MockClusterManagerMockRecorder) CreatePackagesNamespace(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreatePackagesNamespace", reflect.TypeOf((*MockClusterManager)(nil).CreatePackagesNamespace), arg0, arg1) +} + // CreateWorkloadCluster mocks base method. func (m *MockClusterManager) CreateWorkloadCluster(arg0 context.Context, arg1 *types.Cluster, arg2 *cluster.Spec, arg3 providers.Provider) (*types.Cluster, error) { m.ctrl.T.Helper() From 2bf54dce9a422d2d7056e5e6bd3ef91c67f0a440 Mon Sep 17 00:00:00 2001 From: Tanvir Tatla Date: Fri, 1 Dec 2023 09:33:54 -0800 Subject: [PATCH 2/5] remove unneeded field in package controller client --- pkg/curatedpackages/packagecontrollerclient.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/curatedpackages/packagecontrollerclient.go b/pkg/curatedpackages/packagecontrollerclient.go index 41975279be17..3afd0f82ff4c 100644 --- a/pkg/curatedpackages/packagecontrollerclient.go +++ b/pkg/curatedpackages/packagecontrollerclient.go @@ -87,7 +87,6 @@ type PackageControllerClient struct { // registryAccessTester test if the aws credential has access to registry registryAccessTester RegistryAccessTester - flc bool } // ClientBuilder returns a k8s client for the specified cluster. From 43b3f6084b38d428def684cbcc4db51de46cbb5c Mon Sep 17 00:00:00 2001 From: Tanvir Tatla Date: Fri, 1 Dec 2023 12:24:03 -0800 Subject: [PATCH 3/5] fix unit-tests --- controllers/cluster_controller_test.go | 7 +++++++ controllers/cluster_controller_test_test.go | 3 +++ pkg/clustermanager/eksa_installer_test.go | 4 ++-- pkg/workflows/create_test.go | 3 +++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/controllers/cluster_controller_test.go b/controllers/cluster_controller_test.go index f541a62771e1..99bec206af6e 100644 --- a/controllers/cluster_controller_test.go +++ b/controllers/cluster_controller_test.go @@ -182,6 +182,7 @@ func TestClusterReconcilerReconcileSelfManagedCluster(t *testing.T) { registry := newRegistryMock(providerReconciler) c := fake.NewClientBuilder().WithRuntimeObjects(selfManagedCluster, kcp).Build() mockPkgs := mocks.NewMockPackagesClient(controller) + mockPkgs.EXPECT().UpdateSecrets(ctx, c, sameName(selfManagedCluster)) providerReconciler.EXPECT().Reconcile(ctx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(selfManagedCluster)) mhcReconciler.EXPECT().Reconcile(ctx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(selfManagedCluster)).Return(nil) @@ -367,6 +368,7 @@ func TestClusterReconcilerReconcileConditions(t *testing.T) { log := testr.New(t) logCtx := ctrl.LoggerInto(ctx, log) + mockPkgs.EXPECT().UpdateSecrets(logCtx, testClient, sameName(config.Cluster)) iam.EXPECT().EnsureCASecret(logCtx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(config.Cluster)).Return(controller.Result{}, nil) iam.EXPECT().Reconcile(logCtx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(config.Cluster)).Return(controller.Result{}, nil) providerReconciler.EXPECT().Reconcile(logCtx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(config.Cluster)).Times(1) @@ -625,6 +627,8 @@ func TestClusterReconcilerReconcileSelfManagedClusterConditions(t *testing.T) { providerReconciler.EXPECT().Reconcile(gomock.Any(), gomock.Any(), gomock.Any()).Times(1) mhcReconciler.EXPECT().Reconcile(logCtx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(config.Cluster)).Return(nil) + mockPkgs.EXPECT().UpdateSecrets(logCtx, testClient, sameName(config.Cluster)) + r := controllers.NewClusterReconciler(testClient, registry, iam, clusterValidator, mockPkgs, mhcReconciler) result, err := r.Reconcile(logCtx, clusterRequest(config.Cluster)) @@ -772,6 +776,7 @@ func TestClusterReconcilerReconcileGenerations(t *testing.T) { mhcReconciler := mocks.NewMockMachineHealthCheckReconciler(mockCtrl) if tt.wantReconciliation { + mockPkgs.EXPECT().UpdateSecrets(ctx, client, sameName(config.Cluster)) iam.EXPECT().EnsureCASecret(ctx, gomock.AssignableToTypeOf(logr.Logger{}), gomock.AssignableToTypeOf(config.Cluster)).Return(controller.Result{}, nil) iam.EXPECT().Reconcile(ctx, gomock.AssignableToTypeOf(logr.Logger{}), gomock.AssignableToTypeOf(config.Cluster)).Return(controller.Result{}, nil) providerReconciler.EXPECT().Reconcile(ctx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(config.Cluster)).Times(1) @@ -1140,6 +1145,7 @@ func TestClusterReconcilerSkipDontInstallPackagesOnSelfManaged(t *testing.T) { ctrl := gomock.NewController(t) mockPkgs := mocks.NewMockPackagesClient(ctrl) + mockPkgs.EXPECT().UpdateSecrets(ctx, mockClient, sameName(cluster)) mockPkgs.EXPECT().ReconcileDelete(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) mhcReconciler := mocks.NewMockMachineHealthCheckReconciler(ctrl) mhcReconciler.EXPECT().Reconcile(ctx, gomock.Any(), sameName(cluster)).Return(nil) @@ -1318,6 +1324,7 @@ func TestClusterReconcilerPackagesInstall(s *testing.T) { mhcReconciler.EXPECT().Reconcile(logCtx, gomock.AssignableToTypeOf(logr.Logger{}), sameName(cluster)).Return(nil) + mockPkgs.EXPECT().UpdateSecrets(logCtx, fakeClient, sameName(cluster)) mockPkgs.EXPECT(). EnableFullLifecycle(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Times(0) diff --git a/controllers/cluster_controller_test_test.go b/controllers/cluster_controller_test_test.go index 88eafcdecd95..a333e984beda 100644 --- a/controllers/cluster_controller_test_test.go +++ b/controllers/cluster_controller_test_test.go @@ -130,6 +130,7 @@ func TestClusterReconcilerEnsureOwnerReferences(t *testing.T) { validator.EXPECT().ValidateManagementClusterName(ctx, gomock.AssignableToTypeOf(logr.Logger{}), gomock.AssignableToTypeOf(cluster)).Return(nil) pcc := newMockPackagesClient(t) + pcc.EXPECT().UpdateSecrets(ctx, cl, sameName(cluster)) pcc.EXPECT().Reconcile(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mhc := newMockMachineHealthCheckReconciler(t) @@ -312,6 +313,7 @@ func TestClusterReconcilerSetBundlesRef(t *testing.T) { g.Expect(cl.Get(ctx, client.ObjectKey{Namespace: cluster.Spec.BundlesRef.Namespace, Name: cluster.Spec.BundlesRef.Name}, bundles)).To(Succeed()) g.Expect(cl.Get(ctx, client.ObjectKey{Namespace: constants.EksaSystemNamespace, Name: cluster.Name + "-kubeconfig"}, secret)).To(Succeed()) pcc := newMockPackagesClient(t) + pcc.EXPECT().UpdateSecrets(ctx, cl, sameName(cluster)) pcc.EXPECT().Reconcile(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) validator := newMockClusterValidator(t) @@ -366,6 +368,7 @@ func TestClusterReconcilerSetDefaultEksaVersion(t *testing.T) { mgmtCluster := &anywherev1.Cluster{} g.Expect(cl.Get(ctx, client.ObjectKey{Namespace: cluster.Namespace, Name: managementCluster.Name}, mgmtCluster)).To(Succeed()) pcc := newMockPackagesClient(t) + pcc.EXPECT().UpdateSecrets(ctx, cl, sameName(cluster)) pcc.EXPECT().Reconcile(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) validator := newMockClusterValidator(t) diff --git a/pkg/clustermanager/eksa_installer_test.go b/pkg/clustermanager/eksa_installer_test.go index 800f4466e08c..632c852ab373 100644 --- a/pkg/clustermanager/eksa_installer_test.go +++ b/pkg/clustermanager/eksa_installer_test.go @@ -70,7 +70,7 @@ func TestEKSAInstallerInstallSuccessWithRealManifest(t *testing.T) { tt := newInstallerTest(t) tt.newSpec.VersionsBundles["1.19"].Eksa.Components.URI = "../../config/manifest/eksa-components.yaml" tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.AssignableToTypeOf(&appsv1.Deployment{})) - tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.Any()).Times(37) // there are 37 objects in the manifest + tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.Any()).Times(39) // there are 39 objects in the manifest tt.client.EXPECT().WaitForDeployment(tt.ctx, tt.cluster, "30m0s", "Available", "eksa-controller-manager", "eksa-system") tt.Expect(tt.installer.Install(tt.ctx, test.NewNullLogger(), tt.cluster, tt.newSpec)).To(Succeed()) @@ -146,7 +146,7 @@ func TestEKSAInstallerInstallSuccessWithNoTimeout(t *testing.T) { tt := newInstallerTest(t, clustermanager.WithEKSAInstallerNoTimeouts()) tt.newSpec.VersionsBundles["1.19"].Eksa.Components.URI = "../../config/manifest/eksa-components.yaml" tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.AssignableToTypeOf(&appsv1.Deployment{})) - tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.Any()).Times(37) // there are 37 objects in the manifest + tt.client.EXPECT().Apply(tt.ctx, tt.cluster.KubeconfigFile, gomock.Any()).Times(39) // there are 39 objects in the manifest tt.client.EXPECT().WaitForDeployment(tt.ctx, tt.cluster, maxTime.String(), "Available", "eksa-controller-manager", "eksa-system") tt.Expect(tt.installer.Install(tt.ctx, test.NewNullLogger(), tt.cluster, tt.newSpec)).To(Succeed()) diff --git a/pkg/workflows/create_test.go b/pkg/workflows/create_test.go index 8db5517446c5..e9f8b52cfc2e 100644 --- a/pkg/workflows/create_test.go +++ b/pkg/workflows/create_test.go @@ -117,6 +117,9 @@ func (c *createTestSetup) expectCreateWorkload() { c.clusterManager.EXPECT().CreateEKSANamespace( c.ctx, c.workloadCluster, ), + c.clusterManager.EXPECT().CreatePackagesNamespace( + c.ctx, c.workloadCluster, + ), c.clusterManager.EXPECT().InstallCAPI( c.ctx, c.clusterSpec, c.workloadCluster, c.provider, ), From 72075bac9d5fb2b44245c14385f0f5ff1629e3a1 Mon Sep 17 00:00:00 2001 From: Tanvir Tatla Date: Fri, 1 Dec 2023 12:30:43 -0800 Subject: [PATCH 4/5] fix lint warnings --- pkg/clustermanager/cluster_manager.go | 1 + pkg/curatedpackages/packagecontrollerclient.go | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/clustermanager/cluster_manager.go b/pkg/clustermanager/cluster_manager.go index a22d7f0a8745..d718aae5aeb3 100644 --- a/pkg/clustermanager/cluster_manager.go +++ b/pkg/clustermanager/cluster_manager.go @@ -1137,6 +1137,7 @@ func (c *ClusterManager) CreateEKSANamespace(ctx context.Context, cluster *types return c.clusterClient.CreateNamespaceIfNotPresent(ctx, cluster.KubeconfigFile, constants.EksaSystemNamespace) } +// CreatePackagesNamespace creates the eksa-packages namespace on the cluster if it doesn't already exist. func (c *ClusterManager) CreatePackagesNamespace(ctx context.Context, cluster *types.Cluster) error { return c.clusterClient.CreateNamespaceIfNotPresent(ctx, cluster.KubeconfigFile, constants.EksaPackagesName) } diff --git a/pkg/curatedpackages/packagecontrollerclient.go b/pkg/curatedpackages/packagecontrollerclient.go index 3afd0f82ff4c..1b3381976b6b 100644 --- a/pkg/curatedpackages/packagecontrollerclient.go +++ b/pkg/curatedpackages/packagecontrollerclient.go @@ -248,6 +248,7 @@ func (pc *PackageControllerClient) Enable(ctx context.Context) error { return nil } +// UpdateSecrets is used to update the registry-mirror-cred secret used by the packages controller. func (pc *PackageControllerClient) UpdateSecrets(ctx context.Context, client client.Client, cluster *anywherev1.Cluster) error { secretKey := types.NamespacedName{ Namespace: constants.EksaPackagesName, @@ -290,11 +291,11 @@ func fillRegistrySecret(clusterName string, registry *anywherev1.RegistryMirrorC Auth: base64.StdEncoding.EncodeToString([]byte(username + ":" + password)), } - configJson, err := json.Marshal(dconfig) + configJSON, err := json.Marshal(dconfig) if err != nil { return err } - secret.Data["config.json"] = configJson + secret.Data["config.json"] = configJSON return nil } From d04fdf0155e1914de7ec65c9487b039641e93acd Mon Sep 17 00:00:00 2001 From: Tanvir Tatla Date: Fri, 1 Dec 2023 15:10:10 -0800 Subject: [PATCH 5/5] Add unit tests --- .../packagecontrollerclient.go | 25 +++++-- .../packagecontrollerclient_test.go | 68 +++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/pkg/curatedpackages/packagecontrollerclient.go b/pkg/curatedpackages/packagecontrollerclient.go index 1b3381976b6b..d885becbec48 100644 --- a/pkg/curatedpackages/packagecontrollerclient.go +++ b/pkg/curatedpackages/packagecontrollerclient.go @@ -250,11 +250,17 @@ func (pc *PackageControllerClient) Enable(ctx context.Context) error { // UpdateSecrets is used to update the registry-mirror-cred secret used by the packages controller. func (pc *PackageControllerClient) UpdateSecrets(ctx context.Context, client client.Client, cluster *anywherev1.Cluster) error { + secretName := "registry-mirror-cred" secretKey := types.NamespacedName{ Namespace: constants.EksaPackagesName, - Name: "registry-mirror-cred", + Name: secretName, + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: constants.EksaPackagesName, + }, } - secret := &corev1.Secret{} credErr := client.Get(ctx, secretKey, secret) err := fillRegistrySecret(cluster.Name, cluster.Spec.RegistryMirrorConfiguration, secret) if err != nil { @@ -272,13 +278,20 @@ func (pc *PackageControllerClient) UpdateSecrets(ctx context.Context, client cli func fillRegistrySecret(clusterName string, registry *anywherev1.RegistryMirrorConfiguration, secret *corev1.Secret) error { caDataName := clusterName + "_ca.crt" insecureDataName := clusterName + "_insecure" + configName := "config.json" + if secret.Data == nil { + secret.Data = make(map[string][]byte) + } secret.Data[caDataName] = []byte(registry.CACertContent) secret.Data[insecureDataName] = []byte(strconv.FormatBool(registry.InsecureSkipVerify)) dconfig := &dockerConfig{Auths: make(map[string]*dockerAuth)} - err := json.Unmarshal(secret.Data["config.json"], dconfig) - if err != nil { - return err + configData, ok := secret.Data[configName] + if ok { + err := json.Unmarshal(configData, dconfig) + if err != nil { + return err + } } username, password, err := config.ReadCredentials() if err != nil { @@ -295,7 +308,7 @@ func fillRegistrySecret(clusterName string, registry *anywherev1.RegistryMirrorC if err != nil { return err } - secret.Data["config.json"] = configJSON + secret.Data[configName] = configJSON return nil } diff --git a/pkg/curatedpackages/packagecontrollerclient_test.go b/pkg/curatedpackages/packagecontrollerclient_test.go index 1cde163e9651..4b3186eee650 100644 --- a/pkg/curatedpackages/packagecontrollerclient_test.go +++ b/pkg/curatedpackages/packagecontrollerclient_test.go @@ -873,6 +873,74 @@ func TestCreateHelmOverrideValuesYamlFail(t *testing.T) { } } +func TestUpdateSecrets(t *testing.T) { + tests := []struct { + name string + unsetEnv bool + secret *corev1.Secret + error bool + }{ + { + name: "secret_not_found", + secret: nil, + }, + { + name: "secret_found", + secret: &corev1.Secret{}, + }, + { + name: "unmarshal_error", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "config.json": nil, + }, + }, + error: true, + }, + { + name: "no_cred_env", + unsetEnv: true, + error: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + ctrl := gomock.NewController(t) + k := mocks.NewMockKubectlRunner(ctrl) + cm := mocks.NewMockChartManager(ctrl) + kubeConfig := "kubeconfig.kubeconfig" + cluster := newReconcileTestCluster() + cluster.Spec.RegistryMirrorConfiguration = test.RegistryMirrorInsecureSkipVerifyEnabledAndCACert() + objs := []runtime.Object{cluster} + if tt.secret != nil { + tt.secret.Name = "registry-mirror-cred" + tt.secret.Namespace = constants.EksaPackagesName + objs = append(objs, tt.secret) + } + client := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + pcc := curatedpackages.NewPackageControllerClient(cm, k, "test", kubeConfig, nil, nil) + + os.Setenv(constants.RegistryUsername, "test") + os.Setenv(constants.RegistryPassword, "test") + + if tt.unsetEnv { + os.Unsetenv(constants.RegistryUsername) + os.Unsetenv(constants.RegistryPassword) + } + + err := pcc.UpdateSecrets(ctx, client, cluster) + if !tt.error { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err).ToNot(BeNil()) + } + }) + } +} + func TestCreateHelmOverrideValuesYamlFailWithNoWriter(t *testing.T) { for _, tt := range newPackageControllerTests(t) { tt.command = curatedpackages.NewPackageControllerClient(