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.

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

Handle K8s service account lifecycle on eksctl create/delete podidentityassociation commands #7706

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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