From 30fa9335f468d3b22e346f59582d102ad070715d Mon Sep 17 00:00:00 2001 From: Pankti Shah <58618433+panktishah26@users.noreply.github.com> Date: Wed, 5 Feb 2025 19:02:05 -0800 Subject: [PATCH] Validate LicenseToken field is not used by other clusters (#9222) --- pkg/clients/kubernetes/client.go | 21 +++++- pkg/clients/kubernetes/client_test.go | 24 +++++++ pkg/clients/kubernetes/kubeconfig.go | 2 +- pkg/clients/kubernetes/mocks/client.go | 61 +++++++++++++--- pkg/clients/kubernetes/unauth.go | 2 +- pkg/controller/clientutil/kubernetes.go | 12 +++- pkg/controller/clientutil/kubernetes_test.go | 67 +++++++++++++++++ pkg/validations/extendedversion.go | 19 ++++- pkg/validations/extendedversion_test.go | 71 ++++++++++++++++++- pkg/validations/kubectl.go | 2 +- pkg/validations/mocks/kubectl.go | 13 ++-- .../poddisruptionbudgets_test.go | 2 +- 12 files changed, 273 insertions(+), 23 deletions(-) diff --git a/pkg/clients/kubernetes/client.go b/pkg/clients/kubernetes/client.go index 6a7ba233c7af..de3ea9a87774 100644 --- a/pkg/clients/kubernetes/client.go +++ b/pkg/clients/kubernetes/client.go @@ -25,7 +25,26 @@ type Reader interface { // List retrieves list of objects. On a successful call, Items field // in the list will be populated with the result returned from the server. - List(ctx context.Context, list ObjectList) error + List(ctx context.Context, list ObjectList, opts ...ListOption) error +} + +// ListOption is some configuration that modifies options for an apply request. +type ListOption interface { + ApplyToList(*ListOptions) +} + +// ListOptions contains options for listing object. +type ListOptions struct { + // Namespace represents the namespace to list for, or empty for + // non-namespaced objects, or to list across all namespaces. + Namespace string +} + +// ApplyToList implements ApplyToList. +func (o ListOptions) ApplyToList(do *ListOptions) { + if o.Namespace != "" { + do.Namespace = o.Namespace + } } // Writer knows how to create, delete, and update Kubernetes objects. diff --git a/pkg/clients/kubernetes/client_test.go b/pkg/clients/kubernetes/client_test.go index a56c5ee2c242..4e561dc1c374 100644 --- a/pkg/clients/kubernetes/client_test.go +++ b/pkg/clients/kubernetes/client_test.go @@ -8,6 +8,30 @@ import ( "github.com/aws/eks-anywhere/pkg/clients/kubernetes" ) +func TestListOptionsApplyToList(t *testing.T) { + tests := []struct { + name string + option, want *kubernetes.ListOptions + }{ + { + name: "with namespace", + option: &kubernetes.ListOptions{ + Namespace: "test", + }, + want: &kubernetes.ListOptions{ + Namespace: "test", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + tt.option.ApplyToList(tt.option) + g.Expect(tt.option).To(BeComparableTo(tt.want)) + }) + } +} + func TestDeleteAllOfOptionsApplyToDeleteAllOf(t *testing.T) { tests := []struct { name string diff --git a/pkg/clients/kubernetes/kubeconfig.go b/pkg/clients/kubernetes/kubeconfig.go index feb69c303b0e..b85600a9c1a6 100644 --- a/pkg/clients/kubernetes/kubeconfig.go +++ b/pkg/clients/kubernetes/kubeconfig.go @@ -26,7 +26,7 @@ func (c *KubeconfigClient) Get(ctx context.Context, name, namespace string, obj // List retrieves list of objects. On a successful call, Items field // in the list will be populated with the result returned from the server. -func (c *KubeconfigClient) List(ctx context.Context, list ObjectList) error { +func (c *KubeconfigClient) List(ctx context.Context, list ObjectList, _ ...ListOption) error { return c.client.List(ctx, c.kubeconfig, list) } diff --git a/pkg/clients/kubernetes/mocks/client.go b/pkg/clients/kubernetes/mocks/client.go index 82bc91aaec31..c264cdfce8d4 100644 --- a/pkg/clients/kubernetes/mocks/client.go +++ b/pkg/clients/kubernetes/mocks/client.go @@ -116,17 +116,22 @@ func (mr *MockClientMockRecorder) Get(ctx, name, namespace, obj interface{}) *go } // List mocks base method. -func (m *MockClient) List(ctx context.Context, list kubernetes.ObjectList) error { +func (m *MockClient) List(ctx context.Context, list kubernetes.ObjectList, opts ...kubernetes.ListOption) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "List", ctx, list) + varargs := []interface{}{ctx, list} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "List", varargs...) ret0, _ := ret[0].(error) return ret0 } // List indicates an expected call of List. -func (mr *MockClientMockRecorder) List(ctx, list interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) List(ctx, list interface{}, opts ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockClient)(nil).List), ctx, list) + varargs := append([]interface{}{ctx, list}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockClient)(nil).List), varargs...) } // Update mocks base method. @@ -181,17 +186,57 @@ func (mr *MockReaderMockRecorder) Get(ctx, name, namespace, obj interface{}) *go } // List mocks base method. -func (m *MockReader) List(ctx context.Context, list kubernetes.ObjectList) error { +func (m *MockReader) List(ctx context.Context, list kubernetes.ObjectList, opts ...kubernetes.ListOption) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "List", ctx, list) + varargs := []interface{}{ctx, list} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "List", varargs...) ret0, _ := ret[0].(error) return ret0 } // List indicates an expected call of List. -func (mr *MockReaderMockRecorder) List(ctx, list interface{}) *gomock.Call { +func (mr *MockReaderMockRecorder) List(ctx, list interface{}, opts ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, list}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockReader)(nil).List), varargs...) +} + +// MockListOption is a mock of ListOption interface. +type MockListOption struct { + ctrl *gomock.Controller + recorder *MockListOptionMockRecorder +} + +// MockListOptionMockRecorder is the mock recorder for MockListOption. +type MockListOptionMockRecorder struct { + mock *MockListOption +} + +// NewMockListOption creates a new mock instance. +func NewMockListOption(ctrl *gomock.Controller) *MockListOption { + mock := &MockListOption{ctrl: ctrl} + mock.recorder = &MockListOptionMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockListOption) EXPECT() *MockListOptionMockRecorder { + return m.recorder +} + +// ApplyToList mocks base method. +func (m *MockListOption) ApplyToList(arg0 *kubernetes.ListOptions) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "ApplyToList", arg0) +} + +// ApplyToList indicates an expected call of ApplyToList. +func (mr *MockListOptionMockRecorder) ApplyToList(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockReader)(nil).List), ctx, list) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApplyToList", reflect.TypeOf((*MockListOption)(nil).ApplyToList), arg0) } // MockWriter is a mock of Writer interface. diff --git a/pkg/clients/kubernetes/unauth.go b/pkg/clients/kubernetes/unauth.go index d504f047f093..0e97b6c84cee 100644 --- a/pkg/clients/kubernetes/unauth.go +++ b/pkg/clients/kubernetes/unauth.go @@ -90,7 +90,7 @@ func (c *UnAuthClient) ApplyServerSide(ctx context.Context, kubeconfig, fieldMan // List retrieves list of objects. On a successful call, Items field // in the list will be populated with the result returned from the server. -func (c *UnAuthClient) List(ctx context.Context, kubeconfig string, list ObjectList) error { +func (c *UnAuthClient) List(ctx context.Context, kubeconfig string, list ObjectList, _ ...ListOption) error { resourceType, err := c.resourceTypeForObj(list) if err != nil { return fmt.Errorf("getting kubernetes resource: %v", err) diff --git a/pkg/controller/clientutil/kubernetes.go b/pkg/controller/clientutil/kubernetes.go index ed4337bd2a41..81dce4f2f41f 100644 --- a/pkg/controller/clientutil/kubernetes.go +++ b/pkg/controller/clientutil/kubernetes.go @@ -29,8 +29,16 @@ func (c *KubeClient) Get(ctx context.Context, name, namespace string, obj kubern // List retrieves list of objects. On a successful call, Items field // in the list will be populated with the result returned from the server. -func (c *KubeClient) List(ctx context.Context, list kubernetes.ObjectList) error { - return c.client.List(ctx, list) +func (c *KubeClient) List(ctx context.Context, list kubernetes.ObjectList, opts ...kubernetes.ListOption) error { + o := &kubernetes.ListOptions{} + for _, opt := range opts { + opt.ApplyToList(o) + } + + clientOptions := &client.ListOptions{} + clientOptions.Namespace = o.Namespace + + return c.client.List(ctx, list, clientOptions) } // Create saves the object obj in the Kubernetes cluster. diff --git a/pkg/controller/clientutil/kubernetes_test.go b/pkg/controller/clientutil/kubernetes_test.go index 733eca13ec5f..0cf2b6ff3c24 100644 --- a/pkg/controller/clientutil/kubernetes_test.go +++ b/pkg/controller/clientutil/kubernetes_test.go @@ -80,6 +80,73 @@ func TestKubeClientList(t *testing.T) { g.Expect(receiveClusters.Items).To(ConsistOf(*cluster1, *cluster2)) } +func TestKubeClientListOpts(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + cluster1 := &anywherev1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: anywherev1.ClusterKind, + APIVersion: anywherev1.GroupVersion.String(), + }, + } + cluster2 := &anywherev1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster-2", + Namespace: "eksa-system", + }, + TypeMeta: metav1.TypeMeta{ + Kind: anywherev1.ClusterKind, + APIVersion: anywherev1.GroupVersion.String(), + }, + } + cb := fake.NewClientBuilder() + cl := cb.WithRuntimeObjects(cluster1, cluster2).Build() + + client := clientutil.NewKubeClient(cl) + receiveClusters := &anywherev1.ClusterList{} + opts := kubernetes.ListOptions{} + g.Expect(client.List(ctx, receiveClusters, opts)).To(Succeed()) + g.Expect(receiveClusters.Items).To(ConsistOf(*cluster1, *cluster2)) +} + +func TestKubeClientListOptsWithNamespace(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + cluster1 := &anywherev1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: anywherev1.ClusterKind, + APIVersion: anywherev1.GroupVersion.String(), + }, + } + cluster2 := &anywherev1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster-2", + Namespace: "eksa-system", + }, + TypeMeta: metav1.TypeMeta{ + Kind: anywherev1.ClusterKind, + APIVersion: anywherev1.GroupVersion.String(), + }, + } + cb := fake.NewClientBuilder() + cl := cb.WithRuntimeObjects(cluster1, cluster2).Build() + + client := clientutil.NewKubeClient(cl) + receiveClusters := &anywherev1.ClusterList{} + opts := kubernetes.ListOptions{Namespace: "eksa-system"} + g.Expect(client.List(ctx, receiveClusters, opts)).To(Succeed()) + g.Expect(receiveClusters.Items).To(ConsistOf(*cluster2)) + g.Expect(receiveClusters.Items).NotTo(ConsistOf(*cluster1)) +} + func TestKubeClientCreate(t *testing.T) { g := NewWithT(t) ctx := context.Background() diff --git a/pkg/validations/extendedversion.go b/pkg/validations/extendedversion.go index 3848ec3816de..4c13b45afd3d 100644 --- a/pkg/validations/extendedversion.go +++ b/pkg/validations/extendedversion.go @@ -17,7 +17,7 @@ import ( ) // ValidateExtendedK8sVersionSupport validates all the validations needed for the support of extended kubernetes support. -func ValidateExtendedK8sVersionSupport(_ context.Context, clusterSpec anywherev1.Cluster, bundle *v1alpha1.Bundles, _ kubernetes.Client) error { +func ValidateExtendedK8sVersionSupport(ctx context.Context, clusterSpec anywherev1.Cluster, bundle *v1alpha1.Bundles, k kubernetes.Client) error { // Validate EKS-A bundle has not been modified by verifying the signature in the bundle annotation if err := validateBundleSignature(bundle); err != nil { return fmt.Errorf("validating bundle signature: %w", err) @@ -36,6 +36,9 @@ func ValidateExtendedK8sVersionSupport(_ context.Context, clusterSpec anywherev1 if err = validateLicense(token); err != nil { return fmt.Errorf("validating licenseToken: %w", err) } + if err := validateLicenseKeyIsUnique(ctx, clusterSpec.Name, clusterSpec.Spec.LicenseToken, k); err != nil { + return fmt.Errorf("validating licenseToken is unique for cluster %s: %w", clusterSpec.Name, err) + } } return nil } @@ -104,3 +107,17 @@ func isPastDateThanToday(dateToCompare time.Time) bool { today := time.Now().Truncate(24 * time.Hour) return dateToCompare.Before(today) } + +func validateLicenseKeyIsUnique(ctx context.Context, clusterName string, licenseToken string, k kubernetes.Client) error { + eksaClusters := &anywherev1.ClusterList{} + err := k.List(ctx, eksaClusters, kubernetes.ListOptions{}) + if err != nil { + return fmt.Errorf("listing all EKS-A clusters: %w", err) + } + for _, eksaCluster := range eksaClusters.Items { + if eksaCluster.Name != clusterName && eksaCluster.Spec.LicenseToken == licenseToken { + return fmt.Errorf("license key %s is already in use by cluster %s", licenseToken, eksaCluster.Name) + } + } + return nil +} diff --git a/pkg/validations/extendedversion_test.go b/pkg/validations/extendedversion_test.go index f78d5c3d99dc..acc2851453de 100644 --- a/pkg/validations/extendedversion_test.go +++ b/pkg/validations/extendedversion_test.go @@ -1,4 +1,4 @@ -package validations_test +package validations import ( "context" @@ -7,12 +7,13 @@ import ( "testing" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/aws/eks-anywhere/internal/test" anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1" "github.com/aws/eks-anywhere/pkg/clients/kubernetes" "github.com/aws/eks-anywhere/pkg/constants" - "github.com/aws/eks-anywhere/pkg/validations" + "github.com/aws/eks-anywhere/pkg/controller/clientutil" "github.com/aws/eks-anywhere/release/api/v1alpha1" ) @@ -104,7 +105,71 @@ func TestValidateExtendedK8sVersionSupport(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(_ *testing.T) { - err := validations.ValidateExtendedK8sVersionSupport(ctx, tc.cluster, tc.bundle, client) + err := ValidateExtendedK8sVersionSupport(ctx, tc.cluster, tc.bundle, client) + if err != nil && !strings.Contains(err.Error(), tc.wantErr.Error()) { + t.Errorf("%v got = %v, \nwant %v", tc.name, err, tc.wantErr) + } + }) + } +} + +func TestValidateLicenseKeyIsUnique(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + cluster *anywherev1.Cluster + workloadCluster *anywherev1.Cluster + wantErr error + }{ + { + name: "license key is unique", + cluster: &anywherev1.Cluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster1", + }, + Spec: anywherev1.ClusterSpec{ + LicenseToken: "valid-token", + }, + }, + workloadCluster: &anywherev1.Cluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster2", + }, + Spec: anywherev1.ClusterSpec{ + LicenseToken: "valid-token1", + }, + }, + wantErr: nil, + }, + { + name: "license key is not unique", + cluster: &anywherev1.Cluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster1", + }, + Spec: anywherev1.ClusterSpec{ + LicenseToken: "valid-token", + }, + }, + workloadCluster: &anywherev1.Cluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster2", + }, + Spec: anywherev1.ClusterSpec{ + LicenseToken: "valid-token", + }, + }, + wantErr: fmt.Errorf("license key valid-token is already in use by cluster"), + }, + } + for _, tc := range tests { + t.Run(tc.name, func(_ *testing.T) { + cb := fake.NewClientBuilder() + cl := cb.WithRuntimeObjects(tc.cluster, tc.workloadCluster).Build() + client := clientutil.NewKubeClient(cl) + + err := validateLicenseKeyIsUnique(ctx, tc.cluster.Name, tc.cluster.Spec.LicenseToken, client) if err != nil && !strings.Contains(err.Error(), tc.wantErr.Error()) { t.Errorf("%v got = %v, \nwant %v", tc.name, err, tc.wantErr) } diff --git a/pkg/validations/kubectl.go b/pkg/validations/kubectl.go index 5d352565c2cc..14514ab5db1d 100644 --- a/pkg/validations/kubectl.go +++ b/pkg/validations/kubectl.go @@ -16,7 +16,7 @@ import ( ) type KubectlClient interface { - List(ctx context.Context, kubeconfig string, list kubernetes.ObjectList) error + List(ctx context.Context, kubeconfig string, list kubernetes.ObjectList, opts ...kubernetes.ListOption) error ValidateControlPlaneNodes(ctx context.Context, cluster *types.Cluster, clusterName string) error ValidateWorkerNodes(ctx context.Context, clusterName string, kubeconfig string) error ValidateNodes(ctx context.Context, kubeconfig string) error diff --git a/pkg/validations/mocks/kubectl.go b/pkg/validations/mocks/kubectl.go index 1c60831b219e..a269c5a9f95b 100644 --- a/pkg/validations/mocks/kubectl.go +++ b/pkg/validations/mocks/kubectl.go @@ -205,17 +205,22 @@ func (mr *MockKubectlClientMockRecorder) GetObject(ctx, resourceType, name, name } // List mocks base method. -func (m *MockKubectlClient) List(ctx context.Context, kubeconfig string, list kubernetes.ObjectList) error { +func (m *MockKubectlClient) List(ctx context.Context, kubeconfig string, list kubernetes.ObjectList, opts ...kubernetes.ListOption) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "List", ctx, kubeconfig, list) + varargs := []interface{}{ctx, kubeconfig, list} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "List", varargs...) ret0, _ := ret[0].(error) return ret0 } // List indicates an expected call of List. -func (mr *MockKubectlClientMockRecorder) List(ctx, kubeconfig, list interface{}) *gomock.Call { +func (mr *MockKubectlClientMockRecorder) List(ctx, kubeconfig, list interface{}, opts ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockKubectlClient)(nil).List), ctx, kubeconfig, list) + varargs := append([]interface{}{ctx, kubeconfig, list}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockKubectlClient)(nil).List), varargs...) } // SearchIdentityProviderConfig mocks base method. diff --git a/pkg/validations/upgradevalidations/poddisruptionbudgets_test.go b/pkg/validations/upgradevalidations/poddisruptionbudgets_test.go index e5fcc0d6d269..03b8c65da115 100644 --- a/pkg/validations/upgradevalidations/poddisruptionbudgets_test.go +++ b/pkg/validations/upgradevalidations/poddisruptionbudgets_test.go @@ -70,7 +70,7 @@ func TestValidatePodDisruptionBudgets(t *testing.T) { } for _, tt := range tests { podDisruptionBudgets := &policy.PodDisruptionBudgetList{} - k.EXPECT().List(tt.args.ctx, tt.args.cluster.KubeconfigFile, podDisruptionBudgets).DoAndReturn(func(_ context.Context, _ string, objs kubernetes.ObjectList) error { + k.EXPECT().List(tt.args.ctx, tt.args.cluster.KubeconfigFile, podDisruptionBudgets).DoAndReturn(func(_ context.Context, _ string, objs kubernetes.ObjectList, _ ...kubernetes.ListOption) error { tt.args.pdbList.DeepCopyInto(objs.(*policy.PodDisruptionBudgetList)) return nil })