Skip to content

Commit

Permalink
Handle K8s service account lifecycle on `eksctl create/delete podiden…
Browse files Browse the repository at this point in the history
…tityassociation` commands (eksctl-io#7706)

* Handle K8s service account lifecycle on eksctl create/delete podidentityassociations commands

* correct typo

Co-authored-by: Chetan Patwal <[email protected]>

---------

Co-authored-by: Chetan Patwal <[email protected]>
  • Loading branch information
TiberiuGC and cPu1 committed Oct 7, 2024
1 parent 2ba5581 commit 47e061b
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 27 deletions.
10 changes: 9 additions & 1 deletion pkg/actions/cluster/owned.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,15 @@ func (c *OwnedCluster) Delete(ctx context.Context, _, podEvictionWaitPeriod time
}
newTasksToDeleteAddonIAM := addon.NewRemover(c.stackManager).DeleteAddonIAMTasks
newTasksToDeletePodIdentityRoles := func() (*tasks.TaskTree, error) {
return podidentityassociation.NewDeleter(c.cfg.Metadata.Name, c.stackManager, c.ctl.AWSProvider.EKS()).
clientSet, err = c.newClientSet()
if err != nil {
if force {
logger.Warning("error occurred while deleting IAM Role stacks for pod identity associations: %v; force=true so proceeding with cluster deletion", err)
return &tasks.TaskTree{}, nil
}
return nil, err
}
return podidentityassociation.NewDeleter(c.cfg.Metadata.Name, c.stackManager, c.ctl.AWSProvider.EKS(), clientSet).
DeleteTasks(ctx, []podidentityassociation.Identifier{})
}

Expand Down
20 changes: 19 additions & 1 deletion pkg/actions/podidentityassociation/creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ import (
"context"
"fmt"

v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeclient "k8s.io/client-go/kubernetes"

api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
"github.com/weaveworks/eksctl/pkg/awsapi"
"github.com/weaveworks/eksctl/pkg/cfn/builder"
"github.com/weaveworks/eksctl/pkg/kubernetes"
"github.com/weaveworks/eksctl/pkg/utils/tasks"
)

Expand All @@ -21,13 +25,15 @@ type Creator struct {

stackCreator StackCreator
eksAPI awsapi.EKS
clientSet kubeclient.Interface
}

func NewCreator(clusterName string, stackCreator StackCreator, eksAPI awsapi.EKS) *Creator {
func NewCreator(clusterName string, stackCreator StackCreator, eksAPI awsapi.EKS, clientSet kubeclient.Interface) *Creator {
return &Creator{
clusterName: clusterName,
stackCreator: stackCreator,
eksAPI: eksAPI,
clientSet: clientSet,
}
}

Expand All @@ -53,6 +59,18 @@ 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
},
})
piaCreationTasks.Append(&createPodIdentityAssociationTask{
ctx: ctx,
info: fmt.Sprintf("create pod identity association for service account %q", pia.NameString()),
Expand Down
40 changes: 39 additions & 1 deletion pkg/actions/podidentityassociation/creator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import (
"fmt"

awseks "github.com/aws/aws-sdk-go-v2/service/eks"

"k8s.io/apimachinery/pkg/runtime"
kubeclientfakes "k8s.io/client-go/kubernetes/fake"
kubeclienttesting "k8s.io/client-go/testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/mock"
Expand All @@ -20,6 +25,7 @@ type createPodIdentityAssociationEntry struct {
toBeCreated []api.PodIdentityAssociation
mockEKS func(provider *mockprovider.MockProvider)
mockCFN func(stackCreator *fakes.FakeStackCreator)
mockK8s func(clientSet *kubeclientfakes.Clientset)
expectedCreateStackCalls int
expectedErr string
}
Expand All @@ -28,6 +34,7 @@ var _ = Describe("Create", func() {
var (
creator *podidentityassociation.Creator
fakeStackCreator *fakes.FakeStackCreator
fakeClientSet *kubeclientfakes.Clientset
mockProvider *mockprovider.MockProvider

clusterName = "test-cluster"
Expand All @@ -44,12 +51,17 @@ var _ = Describe("Create", func() {
e.mockCFN(fakeStackCreator)
}

fakeClientSet = kubeclientfakes.NewSimpleClientset()
if e.mockK8s != nil {
e.mockK8s(fakeClientSet)
}

mockProvider = mockprovider.NewMockProvider()
if e.mockEKS != nil {
e.mockEKS(mockProvider)
}

creator = podidentityassociation.NewCreator(clusterName, fakeStackCreator, mockProvider.MockEKS())
creator = podidentityassociation.NewCreator(clusterName, fakeStackCreator, mockProvider.MockEKS(), fakeClientSet)

err := creator.CreatePodIdentityAssociations(context.Background(), e.toBeCreated)
if e.expectedErr != "" {
Expand Down Expand Up @@ -80,6 +92,32 @@ var _ = Describe("Create", func() {
expectedErr: "creating IAM role for pod identity association",
}),

Entry("returns an error if creating the service account fails", createPodIdentityAssociationEntry{
toBeCreated: []api.PodIdentityAssociation{
{
Namespace: namespace,
ServiceAccountName: serviceAccountName1,
RoleARN: roleARN,
},
},
mockK8s: func(clientSet *kubeclientfakes.Clientset) {
clientSet.PrependReactor("get", "namespaces", func(action kubeclienttesting.Action) (bool, runtime.Object, error) {
return true, nil, genericErr
})
},
mockEKS: func(provider *mockprovider.MockProvider) {
mockProvider.MockEKS().
On("CreatePodIdentityAssociation", mock.Anything, mock.Anything).
Run(func(args mock.Arguments) {
Expect(args).To(HaveLen(2))
Expect(args[1]).To(BeAssignableToTypeOf(&awseks.CreatePodIdentityAssociationInput{}))
}).
Return(&awseks.CreatePodIdentityAssociationOutput{}, nil).
Once()
},
expectedErr: "failed to create service account",
}),

Entry("returns an error if creating the association fails", createPodIdentityAssociationEntry{
toBeCreated: []api.PodIdentityAssociation{
{
Expand Down
29 changes: 26 additions & 3 deletions pkg/actions/podidentityassociation/deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ import (
"fmt"
"strings"

cfntypes "github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeclient "k8s.io/client-go/kubernetes"

"github.com/aws/aws-sdk-go-v2/aws"
cfntypes "github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
"github.com/aws/aws-sdk-go-v2/service/eks"

"github.com/kris-nova/logger"

api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
"github.com/weaveworks/eksctl/pkg/cfn/manager"
"github.com/weaveworks/eksctl/pkg/kubernetes"
"github.com/weaveworks/eksctl/pkg/utils/tasks"
)

Expand Down Expand Up @@ -49,6 +52,8 @@ type Deleter struct {
StackDeleter StackDeleter
// APIDeleter deletes pod identity associations using the EKS API.
APIDeleter APIDeleter
// ClientSet is used to delete K8s service accounts.
ClientSet kubeclient.Interface
}

// Identifier represents a pod identity association.
Expand All @@ -71,11 +76,12 @@ func (i Identifier) toString(delimiter string) string {
return i.Namespace + delimiter + i.ServiceAccountName
}

func NewDeleter(clusterName string, stackDeleter StackDeleter, apiDeleter APIDeleter) *Deleter {
func NewDeleter(clusterName string, stackDeleter StackDeleter, apiDeleter APIDeleter, clientSet kubeclient.Interface) *Deleter {
return &Deleter{
ClusterName: clusterName,
StackDeleter: stackDeleter,
APIDeleter: apiDeleter,
ClientSet: clientSet,
}
}

Expand Down Expand Up @@ -111,7 +117,24 @@ func (d *Deleter) DeleteTasks(ctx context.Context, podIDs []Identifier) (*tasks.
}

for _, p := range podIDs {
taskTree.Append(d.makeDeleteTask(ctx, p, roleStackNames))
piaDeletionTasks := &tasks.TaskTree{
Parallel: false,
IsSubTask: true,
}
piaDeletionTasks.Append(d.makeDeleteTask(ctx, p, roleStackNames))
piaDeletionTasks.Append(&tasks.GenericTask{
Description: fmt.Sprintf("delete service account %q", p.IDString()),
Doer: func() error {
if err := kubernetes.MaybeDeleteServiceAccount(d.ClientSet, v1.ObjectMeta{
Name: p.ServiceAccountName,
Namespace: p.Namespace,
}); err != nil {
return fmt.Errorf("failed to delete service account %q: %w", p.IDString(), err)
}
return nil
},
})
taskTree.Append(piaDeletionTasks)
}
return taskTree, nil
}
Expand Down
52 changes: 34 additions & 18 deletions pkg/actions/podidentityassociation/deleter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ import (
"crypto/sha1"
"fmt"

cfntypes "github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/eks"
"github.com/stretchr/testify/mock"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
kubeclientfakes "k8s.io/client-go/kubernetes/fake"
kubeclienttesting "k8s.io/client-go/testing"

"github.com/aws/aws-sdk-go-v2/aws"
cfntypes "github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
"github.com/aws/aws-sdk-go-v2/service/eks"
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"

"github.com/weaveworks/eksctl/pkg/actions/podidentityassociation"
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
Expand All @@ -25,7 +30,7 @@ import (
var _ = Describe("Pod Identity Deleter", func() {
type deleteEntry struct {
podIdentityAssociations []api.PodIdentityAssociation
mockCalls func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS)
mockCalls func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS)

expectedCalls func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS)
expectedErr string
Expand All @@ -40,14 +45,23 @@ var _ = Describe("Pod Identity Deleter", func() {
return nil
}
}
mockCalls := func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS, podID podidentityassociation.Identifier) {
mockClientSet := func(clientSet *kubeclientfakes.Clientset) {
clientSet.PrependReactor("delete", "serviceaccounts", func(action kubeclienttesting.Action) (bool, runtime.Object, error) {
return true, nil, nil
})
clientSet.PrependReactor("get", "serviceaccounts", func(action kubeclienttesting.Action) (bool, runtime.Object, error) {
return true, &corev1.ServiceAccount{}, nil
})
}
mockCalls := func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS, podID podidentityassociation.Identifier) {
stackName := makeIRSAv2StackName(podID)
associationID := fmt.Sprintf("%x", sha1.Sum([]byte(stackName)))
mockListPodIdentityAssociations(eksAPI, podID, []ekstypes.PodIdentityAssociationSummary{
{
AssociationId: aws.String(associationID),
},
}, nil)
mockClientSet(clientSet)
eksAPI.On("DeletePodIdentityAssociation", mock.Anything, &eks.DeletePodIdentityAssociationInput{
ClusterName: aws.String(clusterName),
AssociationId: aws.String(associationID),
Expand All @@ -57,12 +71,14 @@ var _ = Describe("Pod Identity Deleter", func() {

DescribeTable("delete pod identity association", func(e deleteEntry) {
provider := mockprovider.NewMockProvider()
clientSet := kubeclientfakes.NewSimpleClientset()
var stackManager managerfakes.FakeStackManager
e.mockCalls(&stackManager, provider.MockEKS())
e.mockCalls(&stackManager, clientSet, provider.MockEKS())
deleter := podidentityassociation.Deleter{
ClusterName: clusterName,
StackDeleter: &stackManager,
APIDeleter: provider.EKS(),
ClientSet: clientSet,
}
err := deleter.Delete(context.Background(), podidentityassociation.ToIdentifiers(e.podIdentityAssociations))

Expand All @@ -80,13 +96,13 @@ var _ = Describe("Pod Identity Deleter", func() {
ServiceAccountName: "default",
},
},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, fakeClientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podID := podidentityassociation.Identifier{
Namespace: "default",
ServiceAccountName: "default",
}
mockListStackNames(stackManager, []podidentityassociation.Identifier{podID})
mockCalls(stackManager, eksAPI, podID)
mockCalls(stackManager, fakeClientSet, eksAPI, podID)
},

expectedCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
Expand All @@ -107,7 +123,7 @@ var _ = Describe("Pod Identity Deleter", func() {
ServiceAccountName: "aws-node",
},
},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podIDs := []podidentityassociation.Identifier{
{
Namespace: "default",
Expand All @@ -120,7 +136,7 @@ var _ = Describe("Pod Identity Deleter", func() {
}
mockListStackNamesWithIRSAv1(stackManager, podIDs[:1], podIDs[1:])
for _, podID := range podIDs {
mockCalls(stackManager, eksAPI, podID)
mockCalls(stackManager, clientSet, eksAPI, podID)
}
},

Expand Down Expand Up @@ -167,7 +183,7 @@ var _ = Describe("Pod Identity Deleter", func() {
ServiceAccountName: "coredns",
},
},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podIDs := []podidentityassociation.Identifier{
{
Namespace: "default",
Expand All @@ -184,7 +200,7 @@ var _ = Describe("Pod Identity Deleter", func() {
}
mockListStackNames(stackManager, podIDs)
for _, podID := range podIDs {
mockCalls(stackManager, eksAPI, podID)
mockCalls(stackManager, clientSet, eksAPI, podID)
}
mockListPodIdentityAssociations(eksAPI, podidentityassociation.Identifier{
Namespace: "kube-system",
Expand All @@ -207,7 +223,7 @@ var _ = Describe("Pod Identity Deleter", func() {
ServiceAccountName: "aws-node",
},
},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podID := podidentityassociation.Identifier{
Namespace: "kube-system",
ServiceAccountName: "aws-node",
Expand Down Expand Up @@ -236,7 +252,7 @@ var _ = Describe("Pod Identity Deleter", func() {
ServiceAccountName: "aws-node",
},
},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podIDs := []podidentityassociation.Identifier{
{
Namespace: "default",
Expand All @@ -263,7 +279,7 @@ var _ = Describe("Pod Identity Deleter", func() {

Entry("delete IAM resources on cluster deletion", deleteEntry{
podIdentityAssociations: []api.PodIdentityAssociation{},
mockCalls: func(stackManager *managerfakes.FakeStackManager, eksAPI *mocksv2.EKS) {
mockCalls: func(stackManager *managerfakes.FakeStackManager, clientSet *kubeclientfakes.Clientset, eksAPI *mocksv2.EKS) {
podIDs := []podidentityassociation.Identifier{
{
Namespace: "default",
Expand Down
2 changes: 1 addition & 1 deletion pkg/actions/podidentityassociation/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (m *Migrator) MigrateToPodIdentity(ctx context.Context, options PodIdentity
}

// add tasks to create pod identity associations
createAssociationsTasks := NewCreator(m.clusterName, nil, m.eksAPI).CreateTasks(ctx, toBeCreated)
createAssociationsTasks := NewCreator(m.clusterName, nil, m.eksAPI, m.clientSet).CreateTasks(ctx, toBeCreated)
if createAssociationsTasks.Len() > 0 {
createAssociationsTasks.IsSubTask = true
taskTree.Append(createAssociationsTasks)
Expand Down
2 changes: 2 additions & 0 deletions pkg/actions/podidentityassociation/migrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ import (

func TestPodIdentityAssociation(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Nodegroup Suite")
RunSpecs(t, "Pod Identity Association Suite")
}
Loading

0 comments on commit 47e061b

Please sign in to comment.