Skip to content

Commit

Permalink
Add option to create service account for pod identities which default…
Browse files Browse the repository at this point in the history
…s to `false` (#7784)

* only create service account if explicitly instructed to do so

* only delete SAs if they were created by eksctl

* fix lint
  • Loading branch information
TiberiuGC committed May 29, 2024
1 parent 2974871 commit 5619e9f
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 46 deletions.
1 change: 1 addition & 0 deletions examples/39-pod-identity-association.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ iam:
# roleARN is given, eksctl will only create the pod identity association
- namespace: default
serviceAccountName: s3-reader
createServiceAccount: true # default is false
roleARN: arn:aws:iam::111122223333:role/role-1

# roleARN is not given, eksctl will first create an IAM role with given roleName using:
Expand Down
26 changes: 14 additions & 12 deletions pkg/actions/podidentityassociation/creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,20 @@ func (c *Creator) CreateTasks(ctx context.Context, podIdentityAssociations []api
stackCreator: c.stackCreator,
})
}
piaCreationTasks.Append(&tasks.GenericTask{
Description: fmt.Sprintf("create service account %q, if it does not already exist", pia.NameString()),
Doer: func() error {
if err := kubernetes.MaybeCreateServiceAccountOrUpdateMetadata(c.clientSet, v1.ObjectMeta{
Name: pia.ServiceAccountName,
Namespace: pia.Namespace,
}); err != nil {
return fmt.Errorf("failed to create service account %q: %w", pia.NameString(), err)
}
return nil
},
})
if pia.CreateServiceAccount {
piaCreationTasks.Append(&tasks.GenericTask{
Description: fmt.Sprintf("create service account %q, if it does not already exist", pia.NameString()),
Doer: func() error {
if err := kubernetes.MaybeCreateServiceAccountOrUpdateMetadata(c.clientSet, v1.ObjectMeta{
Name: pia.ServiceAccountName,
Namespace: pia.Namespace,
}); err != nil {
return fmt.Errorf("failed to create service account %q: %w", pia.NameString(), err)
}
return nil
},
})
}
piaCreationTasks.Append(&createPodIdentityAssociationTask{
ctx: ctx,
info: fmt.Sprintf("create pod identity association for service account %q", pia.NameString()),
Expand Down
7 changes: 4 additions & 3 deletions pkg/actions/podidentityassociation/creator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ var _ = Describe("Create", func() {
Entry("returns an error if creating the service account fails", createPodIdentityAssociationEntry{
toBeCreated: []api.PodIdentityAssociation{
{
Namespace: namespace,
ServiceAccountName: serviceAccountName1,
RoleARN: roleARN,
Namespace: namespace,
ServiceAccountName: serviceAccountName1,
RoleARN: roleARN,
CreateServiceAccount: true,
},
},
mockK8s: func(clientSet *kubeclientfakes.Clientset) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/actions/podidentityassociation/deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (d *Deleter) DeleteTasks(ctx context.Context, podIDs []Identifier) (*tasks.
}
piaDeletionTasks.Append(d.makeDeleteTask(ctx, podID, roleStackNames))
piaDeletionTasks.Append(&tasks.GenericTask{
Description: fmt.Sprintf("delete service account %q", podID.IDString()),
Description: fmt.Sprintf("delete service account %q, if it exists and is managed by eksctl", podID.IDString()),
Doer: func() error {
if err := kubernetes.MaybeDeleteServiceAccount(d.ClientSet, v1.ObjectMeta{
Name: podID.ServiceAccountName,
Expand Down
2 changes: 0 additions & 2 deletions pkg/actions/podidentityassociation/migrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,6 @@ var _ = Describe("Create", func() {
validateCustomLoggerOutput: func(output string) {
Expect(output).To(ContainSubstring("update trust policy for owned role \"test-role-1\""))
Expect(output).To(ContainSubstring("update trust policy for unowned role \"test-role-2\""))
Expect(output).To(ContainSubstring("create service account \"default/service-account-1\", if it does not already exist"))
Expect(output).To(ContainSubstring("create service account \"default/service-account-2\", if it does not already exist"))
Expect(output).To(ContainSubstring("create pod identity association for service account \"default/service-account-1\""))
Expect(output).To(ContainSubstring("create pod identity association for service account \"default/service-account-2\""))
},
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/eksctl.io/v1alpha5/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ type PodIdentityAssociation struct {

RoleARN string `json:"roleARN"`

// +optional
CreateServiceAccount bool `json:"createServiceAccount,omitempty"`

// +optional
RoleName string `json:"roleName,omitempty"`

Expand Down
9 changes: 0 additions & 9 deletions pkg/cfn/manager/create_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ import (
"github.com/weaveworks/eksctl/pkg/vpc"
)

const (
managedByKubernetesLabelKey = "app.kubernetes.io/managed-by"
managedByKubernetesLabelValue = "eksctl"
)

// NewTasksToCreateCluster defines all tasks required to create a cluster along
// with some nodegroups; see CreateAllNodeGroups for how onlyNodeGroupSubset works.
func (c *StackCollection) NewTasksToCreateCluster(ctx context.Context, nodeGroups []*api.NodeGroup,
Expand Down Expand Up @@ -157,10 +152,6 @@ func (c *StackCollection) NewTasksToCreateIAMServiceAccounts(serviceAccounts []*
}
}

if sa.Labels == nil {
sa.Labels = make(map[string]string)
}
sa.Labels[managedByKubernetesLabelKey] = managedByKubernetesLabelValue
if !api.IsEnabled(sa.RoleOnly) {
saTasks.Append(&kubernetesTask{
info: fmt.Sprintf("create serviceaccount %q", sa.NameString()),
Expand Down
2 changes: 1 addition & 1 deletion pkg/ctl/cmdutils/filter/iamserviceaccount_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (f *IAMServiceAccountFilter) SetExcludeExistingFilter(ctx context.Context,
if api.IsEnabled(sa.RoleOnly) {
return nil
}
exists, err := kubernetes.CheckServiceAccountExists(clientSet, sa.ClusterIAMMeta.AsObjectMeta())
exists, _, err := kubernetes.CheckServiceAccountExists(clientSet, sa.ClusterIAMMeta.AsObjectMeta())
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/ctl/create/pod_identity_association.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ func configureCreatePodIdentityAssociationCmd(cmd *cmdutils.Cmd, pia *api.PodIde
fs.StringVar(&pia.RoleName, "role-name", "", "Set a custom name for the created role")
fs.StringVar(&pia.PermissionsBoundaryARN, "permission-boundary-arn", "", "ARN of the policy that is used to set the permission boundary for the role")

fs.BoolVar(&pia.CreateServiceAccount, "create-service-account", false, "instructs eksctl to create the K8s service account")

fs.StringSliceVar(&pia.PermissionPolicyARNs, "permission-policy-arns", []string{}, "List of ARNs of the IAM permission policies to attach")

fs.VarP(&pia.WellKnownPolicies, "well-known-policies", "", "Used to attach common IAM policies")
Expand Down
43 changes: 31 additions & 12 deletions pkg/kubernetes/serviceaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
managedByKubernetesLabelKey = "app.kubernetes.io/managed-by"
managedByKubernetesLabelValue = "eksctl"
)

// NewServiceAccount creates a corev1.ServiceAccount object using the provided meta.
func NewServiceAccount(meta metav1.ObjectMeta) *corev1.ServiceAccount {
return &corev1.ServiceAccount{
Expand All @@ -21,18 +26,25 @@ func NewServiceAccount(meta metav1.ObjectMeta) *corev1.ServiceAccount {
}
}

// CheckServiceAccountExists check if a serviceaccount with a given name already exists, and
// returns boolean or an error
func CheckServiceAccountExists(clientSet Interface, meta metav1.ObjectMeta) (bool, error) {
// CheckServiceAccountExists check if a serviceaccount with a given name already exists,
// and if it is managed by eksctl
func CheckServiceAccountExists(clientSet Interface, meta metav1.ObjectMeta) (bool, bool, error) {
name := meta.Namespace + "/" + meta.Name
_, err := clientSet.CoreV1().ServiceAccounts(meta.Namespace).Get(context.TODO(), meta.Name, metav1.GetOptions{})
if err == nil {
return true, nil
sa, err := clientSet.CoreV1().ServiceAccounts(meta.Namespace).Get(context.TODO(), meta.Name, metav1.GetOptions{})
if err != nil {
if !apierrors.IsNotFound(err) {
return false, false, errors.Wrapf(err, "checking whether serviceaccount %q exists", name)
}
return false, false, nil
}
if !apierrors.IsNotFound(err) {
return false, errors.Wrapf(err, "checking whether serviceaccount %q exists", name)

if sa.Labels != nil {
if value, ok := sa.Labels[managedByKubernetesLabelKey]; ok && (value == managedByKubernetesLabelValue) {
return true, true, nil
}
}
return false, nil

return true, false, nil
}

// MaybeCreateServiceAccountOrUpdateMetadata will only create serviceaccount with the given name if
Expand All @@ -41,11 +53,14 @@ func CheckServiceAccountExists(clientSet Interface, meta metav1.ObjectMeta) (boo
// meta will be retained
func MaybeCreateServiceAccountOrUpdateMetadata(clientSet Interface, meta metav1.ObjectMeta) error {
name := meta.Namespace + "/" + meta.Name

if meta.Labels == nil {
meta.Labels = make(map[string]string)
}
meta.Labels[managedByKubernetesLabelKey] = managedByKubernetesLabelValue
if err := MaybeCreateNamespace(clientSet, meta.Namespace); err != nil {
return err
}
exists, err := CheckServiceAccountExists(clientSet, meta)
exists, _, err := CheckServiceAccountExists(clientSet, meta)
if err != nil {
return err
}
Expand Down Expand Up @@ -100,14 +115,18 @@ func MaybeCreateServiceAccountOrUpdateMetadata(clientSet Interface, meta metav1.
// MaybeDeleteServiceAccount will only delete the serviceaccount if it exists
func MaybeDeleteServiceAccount(clientSet Interface, meta metav1.ObjectMeta) error {
name := meta.Namespace + "/" + meta.Name
exists, err := CheckServiceAccountExists(clientSet, meta)
exists, isManagedByEksctl, err := CheckServiceAccountExists(clientSet, meta)
if err != nil {
return err
}
if !exists {
logger.Info("serviceaccount %q was already deleted", name)
return nil
}
if !isManagedByEksctl {
logger.Info("serviceaccount %q was not created by eksctl; will not be deleted", name)
return nil
}
err = clientSet.CoreV1().ServiceAccounts(meta.Namespace).Delete(context.TODO(), meta.Name, metav1.DeleteOptions{})
if err != nil {
return err
Expand Down
17 changes: 11 additions & 6 deletions pkg/kubernetes/serviceaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ var _ = Describe("Kubernetes serviceaccount object helpers", func() {
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())

ok, err = CheckServiceAccountExists(clientSet, sa)
ok, isManagedByEksctl, err := CheckServiceAccountExists(clientSet, sa)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())
Expect(isManagedByEksctl).To(BeTrue())

{
resp, err := clientSet.CoreV1().ServiceAccounts(sa.Namespace).Get(context.Background(), sa.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())

Expect(resp.Labels).To(BeEmpty())
Expect(resp.Labels).To(HaveKeyWithValue("app.kubernetes.io/managed-by", "eksctl"))
Expect(resp.Annotations).To(BeEmpty())
}

Expand Down Expand Up @@ -122,9 +123,10 @@ var _ = Describe("Kubernetes serviceaccount object helpers", func() {
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())

ok, err = CheckServiceAccountExists(clientSet, sa)
ok, isManagedByEksctl, err := CheckServiceAccountExists(clientSet, sa)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())
Expect(isManagedByEksctl).To(BeTrue())

By("changing an existing value and not touching labels")
sa.Annotations["foo"] = "new"
Expand Down Expand Up @@ -169,24 +171,27 @@ var _ = Describe("Kubernetes serviceaccount object helpers", func() {
err = MaybeCreateServiceAccountOrUpdateMetadata(clientSet, sa)
Expect(err).NotTo(HaveOccurred())

ok, err := CheckServiceAccountExists(clientSet, sa)
ok, isManagedByEksctl, err := CheckServiceAccountExists(clientSet, sa)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())
Expect(isManagedByEksctl).To(BeTrue())

// should delete it
err = MaybeDeleteServiceAccount(clientSet, sa)
Expect(err).NotTo(HaveOccurred())

ok, err = CheckServiceAccountExists(clientSet, sa)
ok, isManagedByEksctl, err = CheckServiceAccountExists(clientSet, sa)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeFalse())
Expect(isManagedByEksctl).To(BeFalse())

// shouldn't fail if it doesn't exist
err = MaybeDeleteServiceAccount(clientSet, sa)
Expect(err).NotTo(HaveOccurred())

ok, err = CheckServiceAccountExists(clientSet, sa)
ok, isManagedByEksctl, err = CheckServiceAccountExists(clientSet, sa)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeFalse())
Expect(isManagedByEksctl).To(BeFalse())
})
})
1 change: 1 addition & 0 deletions userdocs/src/usage/pod-identity-associations.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ iam:
podIdentityAssociations:
- namespace: <string> #required
serviceAccountName: <string> #required
createServiceAccount: true #optional, default is false
roleARN: <string> #required if none of permissionPolicyARNs, permissionPolicy and wellKnownPolicies is specified. Also, cannot be used together with any of the three other referenced fields.
roleName: <string> #optional, generated automatically if not provided, ignored if roleARN is provided
permissionPolicy: {} #optional
Expand Down

0 comments on commit 5619e9f

Please sign in to comment.