refactor oidcMeta/fix irsa controller

api/v1alpha1/irsa_types.go

@@ -20,6 +20,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	// IRSAKind represents the kind attribute of an IRSA resource.
+	IRSAKind = "IRSA"
+)
+
 // IRSASpec defines the desired state of IRSA
 type IRSASpec struct {
 	// Cleanup, when enabled, allows the IRSA to perform garbage collection

config/crd/bases/irsa.kkb0318.github.io_irsas.yaml

@@ -39,6 +39,11 @@ spec:
           spec:
             description: IRSASpec defines the desired state of IRSA
             properties:
+              cleanup:
+                description: |-
+                  Cleanup, when enabled, allows the IRSA to perform garbage collection
+                  of resources that are no longer needed or managed.
+                type: boolean
               iamPolicies:
                 description: IamPolicies represents the list of IAM policies to be
                   attached to the IAM role
@@ -68,6 +73,8 @@ spec:
                       type: string
                     type: array
                 type: object
+            required:
+            - cleanup
             type: object
           status:
             description: IRSAStatus defines the observed state of IRSA

internal/aws/aws_role.go

@@ -12,6 +12,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/iam"
 	"github.com/aws/smithy-go"
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 )
 
 // RoleManager represents the details needed to manage IAM roles
@@ -61,8 +62,8 @@ func (a *AwsIamClient) DeleteIRSARole(ctx context.Context, r RoleManager) error
 }
 
 // CreateIRSARole creates an IAM role with the specified trust policy and attaches specified policies to it
-func (a *AwsIamClient) CreateIRSARole(ctx context.Context, issuerHostPath string, r RoleManager) error {
+func (a *AwsIamClient) CreateIRSARole(ctx context.Context, issuerMeta issuer.OIDCIssuerMeta, r RoleManager) error {
-	providerArn := fmt.Sprintf("arn:aws:iam::%s:oidc-provider/%s", r.AccountId, issuerHostPath)
+	providerArn := fmt.Sprintf("arn:aws:iam::%s:oidc-provider/%s", r.AccountId, issuerMeta.IssuerHostPath())
 	statement := make([]map[string]interface{}, len(r.Namespaces))
 	for i, ns := range r.Namespaces {
 		statement[i] = map[string]interface{}{
@@ -73,7 +74,7 @@ func (a *AwsIamClient) CreateIRSARole(ctx context.Context, issuerHostPath string
 			"Action": "sts:AssumeRoleWithWebIdentity",
 			"Condition": map[string]interface{}{
 				"StringEquals": map[string]interface{}{
-					fmt.Sprintf("%s:sub", issuerHostPath): fmt.Sprintf("system:serviceaccount:%s:%s", ns, r.RoleName),
+					fmt.Sprintf("%s:sub", issuerMeta.IssuerHostPath()): fmt.Sprintf("system:serviceaccount:%s:%s", ns, r.RoleName),
 				},
 			},
 		}

internal/controller/irsa_controller.go

@@ -18,9 +18,11 @@ package controller
 
 import (
 	"context"
+	"fmt"
 
 	awsclient "github.com/kkb0318/irsa-manager/internal/aws"
 	"github.com/kkb0318/irsa-manager/internal/handler"
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 	"github.com/kkb0318/irsa-manager/internal/kubernetes"
 	"github.com/kkb0318/irsa-manager/internal/manifests"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -126,6 +128,19 @@ func (r *IRSAReconciler) reconcileDelete(ctx context.Context, obj *irsav1alpha1.
 }
 
 func (r *IRSAReconciler) reconcile(ctx context.Context, obj *irsav1alpha1.IRSA, kubeClient *kubernetes.KubernetesClient) error {
+	list, err := kubeClient.List(ctx, irsav1alpha1.GroupVersion.WithKind(irsav1alpha1.IRSAKind))
+	if err != nil {
+		return err
+	}
+	if len(list.Items) != 1 {
+		return fmt.Errorf("there should be exactly one IRSASetup item")
+	}
+	irsaSetup := &irsav1alpha1.IRSASetup{}
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(list.Items[0].Object, irsaSetup)
+	if err != nil {
+		return fmt.Errorf("error converting to IRSASetup for %s: %v", list.Items[0].GetName(), err)
+	}
+
 	serviceAccount := obj.Spec.ServiceAccount
 	accountId, err := r.AwsClient.StsClient().GetAccountId()
 	if err != nil {
@@ -137,7 +152,10 @@ func (r *IRSAReconciler) reconcile(ctx context.Context, obj *irsav1alpha1.IRSA,
 		Policies:   obj.Spec.IamPolicies,
 		AccountId:  accountId,
 	}
-	err = r.AwsClient.IamClient().CreateIRSARole(ctx, "", roleManager)
+	err = r.AwsClient.IamClient().CreateIRSARole(ctx,
+		issuer.NewS3IssuerMeta(irsaSetup.Spec.Discovery.S3),
+		roleManager,
+	)
 	if err != nil {
 		return err
 	}

internal/controller/irsa_controller_test.go

@@ -23,6 +23,7 @@ import (
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -33,13 +34,14 @@ import (
 var _ = Describe("IRSA Controller", func() {
 	Context("When reconciling IRSA", func() {
 		tests := []struct {
-			name string
+			name         string
-			objs *irsav1alpha1.IRSA
+			obj          *irsav1alpha1.IRSA
-			f    func(*IRSAReconciler, *irsav1alpha1.IRSA)
+			irsaSetupObj *irsav1alpha1.IRSASetup
+			f            func(*IRSAReconciler, *irsav1alpha1.IRSA)
 		}{
 			{
 				name: "should reconcile successfully",
-				objs: &irsav1alpha1.IRSA{
+				obj: &irsav1alpha1.IRSA{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-resource1",
 						Namespace: "default",
@@ -54,6 +56,7 @@ var _ = Describe("IRSA Controller", func() {
 						},
 					},
 				},
+				irsaSetupObj: newMockIRSASetup(),
 				f: func(r *IRSAReconciler, obj *irsav1alpha1.IRSA) {
 					expected := []expectedResource{
 						{
@@ -98,7 +101,7 @@ var _ = Describe("IRSA Controller", func() {
 			},
 			{
 				name: "error",
-				objs: &irsav1alpha1.IRSA{
+				obj: &irsav1alpha1.IRSA{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-resource2",
 						Namespace: "default",
@@ -113,6 +116,7 @@ var _ = Describe("IRSA Controller", func() {
 						},
 					},
 				},
+				irsaSetupObj: newMockIRSASetup(),
 				f: func(r *IRSAReconciler, obj *irsav1alpha1.IRSA) {
 					expected := []expectedResource{
 						{
@@ -167,24 +171,39 @@ var _ = Describe("IRSA Controller", func() {
 		for _, tt := range tests {
 			It(tt.name, func() {
 				typeNamespacedName := types.NamespacedName{
-					Name:      tt.objs.Name,
+					Name:      tt.obj.Name,
-					Namespace: tt.objs.Namespace,
+					Namespace: tt.obj.Namespace,
 				}
 				controllerReconciler := &IRSAReconciler{
 					Client:    k8sClient,
 					Scheme:    k8sClient.Scheme(),
 					AwsClient: newMockAwsClient(&mockAwsIamAPI{}, nil, nil),
 				}
-				By("creating the custom resource for the Kind IRSASetup")
+				By("creating the mock ISASetup")
-				err := k8sClient.Get(ctx, typeNamespacedName, tt.objs)
+				if tt.irsaSetupObj != nil {
+					err := k8sClient.Get(ctx, client.ObjectKeyFromObject(tt.irsaSetupObj), tt.irsaSetupObj)
+					if err != nil && errors.IsNotFound(err) {
+						Expect(k8sClient.Create(ctx, tt.obj)).To(Succeed())
+					}
+
+				}
+				By("creating the custom resource for the Kind IRSA")
+				err := k8sClient.Get(ctx, typeNamespacedName, tt.obj)
 				if err != nil && errors.IsNotFound(err) {
-					Expect(k8sClient.Create(ctx, tt.objs)).To(Succeed())
+					Expect(k8sClient.Create(ctx, tt.obj)).To(Succeed())
 				}
 				_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
 					NamespacedName: typeNamespacedName,
 				})
 				Expect(err).NotTo(HaveOccurred())
-				tt.f(controllerReconciler, tt.objs)
+				tt.f(controllerReconciler, tt.obj)
+
+				By("deleting the mock ISASetup")
+				err = k8sClient.Get(ctx, client.ObjectKeyFromObject(tt.irsaSetupObj), tt.irsaSetupObj)
+				if err == nil {
+					err = k8sClient.Delete(ctx, tt.irsaSetupObj)
+					Expect(err).NotTo(HaveOccurred())
+				}
 			})
 		}
 		BeforeEach(func() {
@@ -193,3 +212,20 @@ var _ = Describe("IRSA Controller", func() {
 		})
 	})
 })
+
+func newMockIRSASetup() *irsav1alpha1.IRSASetup {
+	return &irsav1alpha1.IRSASetup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Spec: irsav1alpha1.IRSASetupSpec{
+			Discovery: irsav1alpha1.Discovery{
+				S3: irsav1alpha1.S3Discovery{
+					Region:     "ap-northeast-1",
+					BucketName: "irsa-manager-1",
+				},
+			},
+		},
+	}
+}

internal/controller/irsasetup_controller.go

@@ -28,6 +28,7 @@ import (
 	irsav1alpha1 "github.com/kkb0318/irsa-manager/api/v1alpha1"
 	awsclient "github.com/kkb0318/irsa-manager/internal/aws"
 	"github.com/kkb0318/irsa-manager/internal/handler"
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 	"github.com/kkb0318/irsa-manager/internal/kubernetes"
 	"github.com/kkb0318/irsa-manager/internal/manifests"
 	"github.com/kkb0318/irsa-manager/internal/selfhosted"
@@ -153,7 +154,11 @@ func (r *IRSASetupReconciler) reconcileDelete(ctx context.Context, obj *irsav1al
 	if err != nil {
 		return err
 	}
-	return selfhosted.Delete(ctx, factory)
+	return selfhosted.Delete(
+		ctx,
+		factory,
+		issuer.NewS3IssuerMeta(obj.Spec.Discovery.S3),
+	)
 }
 
 // reconcileSelfhosted ensures that the self-hosted resources are set up correctly.
@@ -207,7 +212,12 @@ func reconcileSelfhosted(ctx context.Context, obj *irsav1alpha1.IRSASetup, awsCl
 		string(irsav1alpha1.SelfHostedReasonFailedKeys),
 		string(irsav1alpha1.SelfHostedReasonFailedOidc),
 	)
-	err = selfhosted.Execute(ctx, factory, forceUpdate)
+	err = selfhosted.Execute(
+		ctx,
+		factory,
+		issuer.NewS3IssuerMeta(obj.Spec.Discovery.S3),
+		forceUpdate,
+	)
 	if err != nil {
 		e = err
 		reason = irsav1alpha1.SelfHostedReasonFailedOidc

internal/controller/irsasetup_controller_test.go

@@ -57,7 +57,7 @@ var _ = Describe("IRSASetup Controller", func() {
 						Discovery: irsav1alpha1.Discovery{
 							S3: irsav1alpha1.S3Discovery{
 								Region:     "ap-northeast-1",
-								BucketName: "irsa-manager-kkb-1",
+								BucketName: "irsa-manager-1",
 							},
 						},
 					},
@@ -138,7 +138,7 @@ var _ = Describe("IRSASetup Controller", func() {
 						Discovery: irsav1alpha1.Discovery{
 							S3: irsav1alpha1.S3Discovery{
 								Region:     "ap-northeast-1",
-								BucketName: "irsa-manager-kkb-1",
+								BucketName: "irsa-manager-1",
 							},
 						},
 					},
@@ -223,7 +223,7 @@ var _ = Describe("IRSASetup Controller", func() {
 						Discovery: irsav1alpha1.Discovery{
 							S3: irsav1alpha1.S3Discovery{
 								Region:     "ap-northeast-1",
-								BucketName: "irsa-manager-kkb-1",
+								BucketName: "irsa-manager-1",
 							},
 						},
 					},

internal/issuer/issuer.go

@@ -1,10 +1,12 @@
-package oidc
+package issuer
 
 import (
 	"fmt"
+
+	irsav1alpha1 "github.com/kkb0318/irsa-manager/api/v1alpha1"
 )
 
-type IssuerMeta interface {
+type OIDCIssuerMeta interface {
 	IssuerHostPath() string
 	IssuerUrl() string
 }
@@ -14,8 +16,8 @@ type S3IssuerMeta struct {
 	bucketName string
 }
 
-func NewS3IssuerMeta(region, bucketName string) *S3IssuerMeta {
+func NewS3IssuerMeta(s3 irsav1alpha1.S3Discovery) *S3IssuerMeta {
-	return &S3IssuerMeta{region, bucketName}
+	return &S3IssuerMeta{s3.Region, s3.BucketName}
 }
 
 func (i *S3IssuerMeta) IssuerHostPath() string {

internal/kubernetes/list.go

@@ -0,0 +1,21 @@
+package kubernetes
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// List lists all resources of the given kind.
+func (c KubernetesClient) List(ctx context.Context, gvk schema.GroupVersionKind, listOpts ...client.ListOption) (*unstructured.UnstructuredList, error) {
+	list := &unstructured.UnstructuredList{}
+	list.SetGroupVersionKind(gvk)
+	err := c.client.List(ctx, list, listOpts...)
+	if err != nil {
+		return nil, err
+	}
+
+	return list, nil
+}

internal/selfhosted/oidc.go

@@ -1,11 +1,10 @@
 package selfhosted
 
-import "context"
+import (
+	"context"
 
-type OIDCIssuerMeta interface {
+	"github.com/kkb0318/irsa-manager/internal/issuer"
-	IssuerHostPath() string
+)
-	IssuerUrl() string
-}
 
 type OIDCIdP interface {
 	Create(ctx context.Context) error
@@ -27,8 +26,7 @@ type OIDCIdPDiscovery interface {
 }
 
 type OIDCIdPFactory interface {
-	IssuerMeta() OIDCIssuerMeta
+	IdP(i issuer.OIDCIssuerMeta) (OIDCIdP, error)
-	IdP(i OIDCIssuerMeta) (OIDCIdP, error)
 	IdPDiscovery() OIDCIdPDiscovery
-	IdPDiscoveryContents(i OIDCIssuerMeta) OIDCIdPDiscoveryContents
+	IdPDiscoveryContents(i issuer.OIDCIssuerMeta) OIDCIdPDiscoveryContents
 }

internal/selfhosted/oidc/factory.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	awsclient "github.com/kkb0318/irsa-manager/internal/aws"
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 	"github.com/kkb0318/irsa-manager/internal/selfhosted"
 )
 
@@ -32,11 +33,7 @@ func NewAwsS3IdpFactory(
 	}, nil
 }
 
-func (f *AwsS3IdPFactory) IssuerMeta() selfhosted.OIDCIssuerMeta {
+func (f *AwsS3IdPFactory) IdP(i issuer.OIDCIssuerMeta) (selfhosted.OIDCIdP, error) {
-	return NewS3IssuerMeta(f.region, f.bucketName)
-}
-
-func (f *AwsS3IdPFactory) IdP(i selfhosted.OIDCIssuerMeta) (selfhosted.OIDCIdP, error) {
 	return NewAwsIdP(f.awsClient, i)
 }
 
@@ -44,6 +41,6 @@ func (f *AwsS3IdPFactory) IdPDiscovery() selfhosted.OIDCIdPDiscovery {
 	return NewS3IdPDiscovery(f.awsClient, f.region, f.bucketName)
 }
 
-func (f *AwsS3IdPFactory) IdPDiscoveryContents(i selfhosted.OIDCIssuerMeta) selfhosted.OIDCIdPDiscoveryContents {
+func (f *AwsS3IdPFactory) IdPDiscoveryContents(i issuer.OIDCIssuerMeta) selfhosted.OIDCIdPDiscoveryContents {
 	return NewIdPDiscoveryContents(f.jwk, i, f.jwksFileName)
 }

internal/selfhosted/oidc/id_provider.go

@@ -4,16 +4,16 @@ import (
 	"context"
 
 	awsclient "github.com/kkb0318/irsa-manager/internal/aws"
-	"github.com/kkb0318/irsa-manager/internal/selfhosted"
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 )
 
 type AwsIdP struct {
 	iamClient  *awsclient.AwsIamClient
 	stsClient  *awsclient.AwsStsClient
-	issuerMeta selfhosted.OIDCIssuerMeta
+	issuerMeta issuer.OIDCIssuerMeta
 }
 
-func NewAwsIdP(awsConfig awsclient.AwsClient, issuerMeta selfhosted.OIDCIssuerMeta) (*AwsIdP, error) {
+func NewAwsIdP(awsConfig awsclient.AwsClient, issuerMeta issuer.OIDCIssuerMeta) (*AwsIdP, error) {
 	iamClient := awsConfig.IamClient()
 	stsClient := awsConfig.StsClient()
 	return &AwsIdP{iamClient, stsClient, issuerMeta}, nil

internal/selfhosted/oidc/id_provider_discovery_contents.go

@@ -4,12 +4,13 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 	"github.com/kkb0318/irsa-manager/internal/selfhosted"
 )
 
 type IdPDiscoveryContents struct {
 	jwk          *selfhosted.JWK
-	issuerMeta   selfhosted.OIDCIssuerMeta
+	issuerMeta   issuer.OIDCIssuerMeta
 	jwksFileName string
 }
 
@@ -23,7 +24,7 @@ type oidcDiscoveryConfiguration struct {
 	ClaimsSupported                  []string `json:"claims_supported"`
 }
 
-func NewIdPDiscoveryContents(jwk *selfhosted.JWK, issuerMeta selfhosted.OIDCIssuerMeta, jwksFileName string) *IdPDiscoveryContents {
+func NewIdPDiscoveryContents(jwk *selfhosted.JWK, issuerMeta issuer.OIDCIssuerMeta, jwksFileName string) *IdPDiscoveryContents {
 	return &IdPDiscoveryContents{jwk, issuerMeta, jwksFileName}
 }
 

internal/selfhosted/selfhosted.go

@@ -2,10 +2,11 @@ package selfhosted
 
 import (
 	"context"
+
+	"github.com/kkb0318/irsa-manager/internal/issuer"
 )
 
-func Execute(ctx context.Context, idpComponentsFactory OIDCIdPFactory, forceUpdate bool) error {
+func Execute(ctx context.Context, idpComponentsFactory OIDCIdPFactory, issuerMeta issuer.OIDCIssuerMeta, forceUpdate bool) error {
-	issuerMeta := idpComponentsFactory.IssuerMeta()
 	discovery := idpComponentsFactory.IdPDiscovery()
 	discoveryContents := idpComponentsFactory.IdPDiscoveryContents(issuerMeta)
 	idp, err := idpComponentsFactory.IdP(issuerMeta)
@@ -27,8 +28,7 @@ func Execute(ctx context.Context, idpComponentsFactory OIDCIdPFactory, forceUpda
 	return nil
 }
 
-func Delete(ctx context.Context, factory OIDCIdPFactory) error {
+func Delete(ctx context.Context, factory OIDCIdPFactory, issuerMeta issuer.OIDCIssuerMeta) error {
-	issuerMeta := factory.IssuerMeta()
 	discovery := factory.IdPDiscovery()
 	discoveryContents := factory.IdPDiscoveryContents(issuerMeta)
 	idp, err := factory.IdP(issuerMeta)