Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS IAM integration - label service account instead of creating it. #92

Merged
merged 1 commit into from
Nov 17, 2023
Merged

AWS IAM integration - label service account instead of creating it. #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/operator/controllers/metadata/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
56 changes: 9 additions & 47 deletions src/operator/controllers/service_account_pod/service_account_pod_reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,22 @@ 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"
"sigs.k8s.io/controller-runtime/pkg/controller"
)

const (
ReasonServiceAccountCreated = "ServiceAccountCreated"
ReasonGetServiceAccountFailed = "GetServiceAccountFailed"
ReasonCreatingServiceAccountFailed = "ServiceAccountCreationFailed"
ReasonServiceAccountUpdated = "ServiceAccountUpdated"
ReasonServiceAccountUpdateFailed = "ServiceAccountUpdateFailed"
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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)
}
100 changes: 10 additions & 90 deletions src/operator/controllers/service_account_pod/service_account_pod_reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -57,19 +56,18 @@ 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(
&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}})
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{})).
Expand All @@ -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,
Expand All @@ -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{})).
Expand All @@ -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{})).
Expand All @@ -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))
}
Loading