diff --git a/src/operator/controllers/metadata/annotations.go b/src/operator/controllers/metadata/annotations.go index 14eff8a..939828d 100644 --- a/src/operator/controllers/metadata/annotations.go +++ b/src/operator/controllers/metadata/annotations.go @@ -10,8 +10,8 @@ const ( TLSSecretNameAnnotation = "credentials-operator.otterize.com/tls-secret-name" TLSSecretNameAnnotationDeprecated = "spire-integration.otterize.com/tls-secret-name" - // ServiceAccountNameAnnotation is the name of the K8s service account that the operator will create - ServiceAccountNameAnnotation = "credentials-operator.otterize.com/service-account-name" + // CreateAWSRoleAnnotation by using this annotation a pod marks that the operator should create an AWS IAM role for its service account + CreateAWSRoleAnnotation = "credentials-operator.otterize.com/create-aws-role" // ServiceAccountAWSRoleARNAnnotation is used by EKS (Kubernetes at AWS) to link between service accounts // and IAM roles ServiceAccountAWSRoleARNAnnotation = "eks.amazonaws.com/role-arn" diff --git a/src/operator/controllers/service_account_pod/service_account_pod_reconciler.go b/src/operator/controllers/service_account_pod/service_account_pod_reconciler.go index fa81ac0..31bdd27 100644 --- a/src/operator/controllers/service_account_pod/service_account_pod_reconciler.go +++ b/src/operator/controllers/service_account_pod/service_account_pod_reconciler.go @@ -2,17 +2,14 @@ package service_account_pod import ( "context" - "fmt" "github.com/otterize/credentials-operator/src/controllers/metadata" "github.com/otterize/intents-operator/src/shared/awsagent" "github.com/samber/lo" "github.com/sirupsen/logrus" v1 "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/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,7 +17,7 @@ import ( ) const ( - ReasonServiceAccountCreated = "ServiceAccountCreated" + ReasonGetServiceAccountFailed = "GetServiceAccountFailed" ReasonCreatingServiceAccountFailed = "ServiceAccountCreationFailed" ReasonServiceAccountUpdated = "ServiceAccountUpdated" ReasonServiceAccountUpdateFailed = "ServiceAccountUpdateFailed" @@ -49,10 +46,6 @@ func (r *PodServiceAccountReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func isServiceAccountNameValid(name string) bool { - return len(validation.IsDNS1123Subdomain(name)) == 0 -} - func (e *PodServiceAccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { var pod v1.Pod err := e.client.Get(ctx, req.NamespacedName, &pod) @@ -63,45 +56,28 @@ func (e *PodServiceAccountReconciler) Reconcile(ctx context.Context, req ctrl.Re if pod.Annotations == nil { return ctrl.Result{}, nil } - serviceAccountName, annotationExists := pod.Annotations[metadata.ServiceAccountNameAnnotation] + _, annotationExists := pod.Annotations[metadata.CreateAWSRoleAnnotation] if !annotationExists { - logrus.Debugf("pod %v doesn't have service account annotation, skipping ensure service account", pod) + logrus.Debugf("pod %v doesn't have create AWS IAM role annotation, skipping", pod) return ctrl.Result{}, nil } - if !isServiceAccountNameValid(serviceAccountName) { - err := fmt.Errorf("service account name %s is invalid according to 'RFC 1123 subdomain'. skipping service account ensure for pod %v", serviceAccountName, pod) - e.recorder.Eventf(&pod, v1.EventTypeWarning, ReasonCreatingServiceAccountFailed, err.Error()) - return ctrl.Result{}, err - } - serviceAccount := v1.ServiceAccount{} - err = e.client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: serviceAccountName}, &serviceAccount) - - if apierrors.IsNotFound(err) { - logrus.Debugf("creating service account named %s for pod/%s/%s", serviceAccountName, pod.Namespace, pod.Name) - if err := e.createServiceAccount(ctx, serviceAccountName, &pod); err != nil { - e.recorder.Eventf(&pod, v1.EventTypeWarning, ReasonCreatingServiceAccountFailed, "Failed creating service account: %s", err.Error()) - return ctrl.Result{}, err - } - e.recorder.Eventf(&pod, v1.EventTypeNormal, ReasonServiceAccountCreated, "Successfully created service account: %s", serviceAccountName) - logrus.Debugf("successfuly created service account named %s for pod/%s/%s", serviceAccountName, pod.Namespace, pod.Name) - return ctrl.Result{}, nil - } + err = e.client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: pod.Spec.ServiceAccountName}, &serviceAccount) if err != nil { - e.recorder.Eventf(&pod, v1.EventTypeWarning, ReasonCreatingServiceAccountFailed, "Failed creating service account: %s", err.Error()) + e.recorder.Eventf(&pod, v1.EventTypeWarning, ReasonGetServiceAccountFailed, "Failed getting service account: %s for pod: %v", pod.Spec.ServiceAccountName, pod) return ctrl.Result{}, err } - logrus.Debugf("service account %s already exists, Updating it", serviceAccountName) + logrus.Debugf("service account %s exists, Updating it", pod.Spec.ServiceAccountName) updatedServiceAccount := serviceAccount.DeepCopy() if updatedServiceAccount.Labels == nil { updatedServiceAccount.Labels = make(map[string]string) } previousServiceAccountValue, ok := serviceAccount.Labels[metadata.OtterizeServiceAccountLabel] - if !ok || previousServiceAccountValue != serviceAccountName { - updatedServiceAccount.Labels[metadata.OtterizeServiceAccountLabel] = serviceAccountName + if !ok || previousServiceAccountValue != pod.Spec.ServiceAccountName { + updatedServiceAccount.Labels[metadata.OtterizeServiceAccountLabel] = pod.Spec.ServiceAccountName err := e.client.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount)) if err != nil { if apierrors.IsConflict(err) { @@ -110,22 +86,8 @@ func (e *PodServiceAccountReconciler) Reconcile(ctx context.Context, req ctrl.Re e.recorder.Eventf(&pod, v1.EventTypeWarning, ReasonServiceAccountUpdateFailed, "failed to update pre-existing service account with Otterize label: %s", err.Error()) return ctrl.Result{}, err } - e.recorder.Eventf(&pod, v1.EventTypeNormal, ReasonServiceAccountUpdated, "service account %s already exists but is missing Otterize label, labeling", serviceAccountName) + e.recorder.Eventf(&pod, v1.EventTypeNormal, ReasonServiceAccountUpdated, "service account %s already exists updated labels, labeling", pod.Spec.ServiceAccountName) } return ctrl.Result{}, nil } - -func (e *PodServiceAccountReconciler) createServiceAccount(ctx context.Context, serviceAccountName string, pod *v1.Pod) error { - serviceAccount := v1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: serviceAccountName, - Namespace: pod.Namespace, - Labels: map[string]string{ - metadata.OtterizeServiceAccountLabel: serviceAccountName, - }, - }, - } - - return e.client.Create(ctx, &serviceAccount) -} diff --git a/src/operator/controllers/service_account_pod/service_account_pod_reconciler_test.go b/src/operator/controllers/service_account_pod/service_account_pod_reconciler_test.go index 71c3850..e42b0af 100644 --- a/src/operator/controllers/service_account_pod/service_account_pod_reconciler_test.go +++ b/src/operator/controllers/service_account_pod/service_account_pod_reconciler_test.go @@ -2,7 +2,6 @@ package service_account_pod import ( "context" - "errors" "fmt" "github.com/otterize/credentials-operator/src/controllers/metadata" mock_client "github.com/otterize/credentials-operator/src/mocks/controller-runtime/client" @@ -57,9 +56,9 @@ func (s *PodServiceAccountEnsurerSuite) SetupTest() { s.ServiceAccountReconciler = NewPodServiceAccountReconciler(s.client, scheme, s.mockEventRecorder, nil) } -func (s *PodServiceAccountEnsurerSuite) TestCreate() { +func (s *PodServiceAccountEnsurerSuite) TestErrorWhenSADoesntExist() { serviceAccountName := "cool.name" - annotations := map[string]string{metadata.ServiceAccountNameAnnotation: serviceAccountName} + annotations := map[string]string{metadata.CreateAWSRoleAnnotation: serviceAccountName} namespace := "namespace" s.client.EXPECT().Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: serviceAccountName, Namespace: namespace}), gomock.AssignableToTypeOf(&v1.ServiceAccount{})). Return( @@ -67,9 +66,8 @@ func (s *PodServiceAccountEnsurerSuite) TestCreate() { ErrStatus: metav1.Status{Status: metav1.StatusFailure, Code: http.StatusNotFound, Reason: metav1.StatusReasonNotFound}, }) - s.client.EXPECT().Create(gomock.Any(), &serviceAccountMatcher{Name: serviceAccountName, Namespace: namespace, Labels: map[string]string{metadata.OtterizeServiceAccountLabel: serviceAccountName}}) - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeNormal), gomock.Eq(ReasonServiceAccountCreated), gomock.Any(), gomock.Any()) - pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}} + s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonGetServiceAccountFailed), gomock.Any(), gomock.Any()) + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}, Spec: v1.PodSpec{ServiceAccountName: serviceAccountName}} s.client.EXPECT(). Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). @@ -78,14 +76,14 @@ func (s *PodServiceAccountEnsurerSuite) TestCreate() { }) res, err := s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().NoError(err) + s.Require().Error(err) s.Require().True(res.IsZero()) } func (s *PodServiceAccountEnsurerSuite) TestUpdateWhenFound() { serviceAccountName := "cool.name" - annotations := map[string]string{metadata.ServiceAccountNameAnnotation: serviceAccountName} + annotations := map[string]string{metadata.CreateAWSRoleAnnotation: serviceAccountName} namespace := "namespace" serviceAccount := v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{ Name: serviceAccountName, @@ -96,7 +94,9 @@ func (s *PodServiceAccountEnsurerSuite) TestUpdateWhenFound() { s.client.EXPECT().Patch(gomock.Any(), &serviceAccountMatcher{Name: serviceAccountName, Namespace: namespace, Labels: map[string]string{metadata.OtterizeServiceAccountLabel: serviceAccountName}}, gomock.AssignableToTypeOf(client.MergeFrom(&serviceAccount))) s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeNormal), gomock.Eq(ReasonServiceAccountUpdated), gomock.Any(), gomock.Any()) - pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}} + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}, Spec: v1.PodSpec{ + ServiceAccountName: serviceAccountName, + }} s.client.EXPECT(). Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). @@ -110,46 +110,7 @@ func (s *PodServiceAccountEnsurerSuite) TestUpdateWhenFound() { } -func (s *PodServiceAccountEnsurerSuite) TestDoesntCreateWhenInvalidName() { - // Name with caps RFC 1123 subdomain - annotations := map[string]string{metadata.ServiceAccountNameAnnotation: "NameWithCapitalLetters"} - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonCreatingServiceAccountFailed), gomock.Any()) - pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: "namespace", Annotations: annotations}} - s.client.EXPECT(). - Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). - Do(func(_, _ any, podPtr *v1.Pod, _ ...interface{}) { - *podPtr = pod - }) - _, err := s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().Error(err) - - // Very long Name (>253) - annotations = map[string]string{metadata.ServiceAccountNameAnnotation: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"} - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonCreatingServiceAccountFailed), gomock.Any()) - pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: "namespace", Annotations: annotations}} - s.client.EXPECT(). - Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). - Do(func(_, _ any, podPtr *v1.Pod, _ ...interface{}) { - *podPtr = pod - }) - _, err = s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().Error(err) - - // Name with / - annotations = map[string]string{metadata.ServiceAccountNameAnnotation: "name/asd"} - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonCreatingServiceAccountFailed), gomock.Any()) - pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: "namespace", Annotations: annotations}} - s.client.EXPECT(). - Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). - Do(func(_, _ any, podPtr *v1.Pod, _ ...interface{}) { - *podPtr = pod - }) - _, err = s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().Error(err) - -} - -func (s *PodServiceAccountEnsurerSuite) TestDoesntCreateWhenNoAnnotation() { +func (s *PodServiceAccountEnsurerSuite) TestDoNothingWhenNoAnnotation() { pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: "namespace"}} s.client.EXPECT(). Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). @@ -161,47 +122,6 @@ func (s *PodServiceAccountEnsurerSuite) TestDoesntCreateWhenNoAnnotation() { s.Require().True(res.IsZero()) } -func (s *PodServiceAccountEnsurerSuite) TestEventOnErrorListing() { - serviceAccountName := "cool.name" - annotations := map[string]string{metadata.ServiceAccountNameAnnotation: serviceAccountName} - namespace := "namespace" - s.client.EXPECT().Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: serviceAccountName, Namespace: namespace}), gomock.AssignableToTypeOf(&v1.ServiceAccount{})). - Return(errors.New("unexpected error")) - - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonCreatingServiceAccountFailed), gomock.Any(), gomock.Any()) - pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}} - s.client.EXPECT(). - Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). - Do(func(_, _ any, podPtr *v1.Pod, _ ...interface{}) { - *podPtr = pod - }) - _, err := s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().Error(err) - -} - -func (s *PodServiceAccountEnsurerSuite) TestEventOnErrorCreating() { - serviceAccountName := "cool.name" - annotations := map[string]string{metadata.ServiceAccountNameAnnotation: serviceAccountName} - namespace := "namespace" - s.client.EXPECT().Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: serviceAccountName, Namespace: namespace}), gomock.AssignableToTypeOf(&v1.ServiceAccount{})). - Return( - &k8serrors.StatusError{ - ErrStatus: metav1.Status{Status: metav1.StatusFailure, Code: http.StatusNotFound, Reason: metav1.StatusReasonNotFound}, - }) - - s.client.EXPECT().Create(gomock.Any(), &serviceAccountMatcher{Name: serviceAccountName, Namespace: namespace, Labels: map[string]string{metadata.OtterizeServiceAccountLabel: serviceAccountName}}).Return(errors.New("unexpected error")) - s.mockEventRecorder.EXPECT().Eventf(gomock.Any(), gomock.Eq(v1.EventTypeWarning), gomock.Eq(ReasonCreatingServiceAccountFailed), gomock.Any(), gomock.Any()) - pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "Pod", Namespace: namespace, Annotations: annotations}} - s.client.EXPECT(). - Get(gomock.Any(), gomock.Eq(types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}), gomock.AssignableToTypeOf(&v1.Pod{})). - Do(func(_, _ any, podPtr *v1.Pod, _ ...interface{}) { - *podPtr = pod - }) - _, err := s.ServiceAccountReconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}}) - s.Require().Error(err) -} - func TestPodServiceAccountEnsurerSuite(t *testing.T) { suite.Run(t, new(PodServiceAccountEnsurerSuite)) }