From 88d480af7064358266302f4afecee8fd6acf3b04 Mon Sep 17 00:00:00 2001 From: Mario Nitchev Date: Mon, 30 Sep 2024 15:55:55 +0300 Subject: [PATCH] Enable share reconciler (#216) * Enable share reconciler * update CHANGELOG * Use management cluster account to add to prefix list entries Shared prefix lists cannot be modified --- CHANGELOG.md | 4 ++ controllers/prefix_list_entry.go | 2 +- controllers/prefix_list_entry_test.go | 4 +- controllers/share.go | 90 +++++++++++++++++++++------ controllers/share_test.go | 10 ++- main.go | 26 +++++--- 6 files changed, 104 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4a64d5d..adc910eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Enable ShareReconciler. + ## [0.16.0] - 2024-08-21 ### Changed diff --git a/controllers/prefix_list_entry.go b/controllers/prefix_list_entry.go index 570b0f64..5d7b5d48 100644 --- a/controllers/prefix_list_entry.go +++ b/controllers/prefix_list_entry.go @@ -94,7 +94,7 @@ func (r *PrefixListEntryReconciler) Reconcile(ctx context.Context, req ctrl.Requ logger = logger.WithValues("prefix-list-arn", prefixListARN) log.IntoContext(ctx, logger) - identity, err := r.clusterClient.GetIdentity(ctx, cluster) + identity, err := r.clusterClient.GetIdentity(ctx, managementCluster) if err != nil { logger.Error(err, "failed to get cluster identity") return ctrl.Result{}, errors.WithStack(err) diff --git a/controllers/prefix_list_entry_test.go b/controllers/prefix_list_entry_test.go index eccf1d44..ffd5129a 100644 --- a/controllers/prefix_list_entry_test.go +++ b/controllers/prefix_list_entry_test.go @@ -56,12 +56,12 @@ var _ = Describe("PrefixListEntryReconciler", func() { BeforeEach(func() { ctx = context.Background() prefixListARN = fmt.Sprintf("arn:aws:iam::123456789012:managed-prefix-lists/%s", uuid.NewString()) - identity, cluster = createRandomClusterWithIdentity( + cluster = createRandomCluster( annotation.NetworkTopologyModeAnnotation, annotation.NetworkTopologyModeGiantSwarmManaged, ) - managementCluster = createRandomCluster( + identity, managementCluster = createRandomClusterWithIdentity( annotation.NetworkTopologyModeAnnotation, annotation.NetworkTopologyModeGiantSwarmManaged, annotation.NetworkTopologyPrefixListIDAnnotation, diff --git a/controllers/share.go b/controllers/share.go index e7d878d5..164f50db 100644 --- a/controllers/share.go +++ b/controllers/share.go @@ -26,7 +26,7 @@ const ( ) type ShareReconciler struct { - ramClient resolver.RAMClient + awsClients resolver.AWSClients clusterClient AWSClusterClient managementCluster k8stypes.NamespacedName } @@ -34,10 +34,10 @@ type ShareReconciler struct { func NewShareReconciler( managementCluster types.NamespacedName, clusterClient AWSClusterClient, - ramClient resolver.RAMClient, + awsClients resolver.AWSClients, ) *ShareReconciler { return &ShareReconciler{ - ramClient: ramClient, + awsClients: awsClients, clusterClient: clusterClient, managementCluster: managementCluster, } @@ -75,25 +75,59 @@ func (r *ShareReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return r.reconcileNormal(ctx, cluster) } +func (r *ShareReconciler) getRamClient(ctx context.Context) (resolver.RAMClient, error) { + logger := log.FromContext(ctx) + + managementCluster, err := r.clusterClient.GetAWSCluster(ctx, r.managementCluster) + if err != nil { + logger.Error(err, "failed to get management cluster") + return nil, errors.WithStack(err) + } + + return r.getRamClientFromCluster(ctx, managementCluster) +} + +func (r *ShareReconciler) getRamClientFromCluster(ctx context.Context, cluster *capa.AWSCluster) (resolver.RAMClient, error) { + logger := log.FromContext(ctx) + + identity, err := r.clusterClient.GetIdentity(ctx, cluster) + if err != nil { + logger.Error(err, "Failed to get cluster identity") + return nil, errors.WithStack(err) + } + + ramClient, err := r.awsClients.NewRAMClient(cluster.Spec.Region, identity.Spec.RoleArn) + if err != nil { + logger.Error(err, "Failed to create ram client") + return nil, errors.WithStack(err) + } + + return ramClient, err +} + func (r *ShareReconciler) reconcileDelete(ctx context.Context, cluster *capa.AWSCluster) (ctrl.Result, error) { if !controllerutil.ContainsFinalizer(cluster, FinalizerResourceShare) { return ctrl.Result{}, nil } logger := log.FromContext(ctx) + ramClient, err := r.getRamClient(ctx) + if err != nil { + return ctrl.Result{}, err + } if resourcesStillInUse(cluster) { logger.Info("Transit gateway and prefix list not yet cleaned up. Skipping...") return ctrl.Result{}, nil } - err := r.ramClient.DeleteResourceShare(ctx, getTransitGatewayResourceShareName(cluster)) + err = ramClient.DeleteResourceShare(ctx, getTransitGatewayResourceShareName(cluster)) if err != nil { logger.Error(err, "failed to delete resource share") return ctrl.Result{}, err } - err = r.ramClient.DeleteResourceShare(ctx, getPrefixListResourceShareName(cluster)) + err = ramClient.DeleteResourceShare(ctx, getPrefixListResourceShareName(cluster)) if err != nil { logger.Error(err, "failed to delete resource share") return ctrl.Result{}, err @@ -108,6 +142,13 @@ func (r *ShareReconciler) reconcileDelete(ctx context.Context, cluster *capa.AWS return ctrl.Result{}, nil } +type shareScope struct { + cluster *capa.AWSCluster + managementCluster *capa.AWSCluster + accountID string + ramClient resolver.RAMClient +} + func (r *ShareReconciler) reconcileNormal(ctx context.Context, cluster *capa.AWSCluster) (ctrl.Result, error) { logger := log.FromContext(ctx) accountID, err := r.getAccountId(ctx, cluster) @@ -118,15 +159,26 @@ func (r *ShareReconciler) reconcileNormal(ctx context.Context, cluster *capa.AWS managementCluster, err := r.clusterClient.GetAWSCluster(ctx, r.managementCluster) if err != nil { logger.Error(err, "failed to get management cluster") - return ctrl.Result{}, errors.WithStack(err) + return ctrl.Result{}, err } + ramClient, err := r.getRamClientFromCluster(ctx, managementCluster) + if err != nil { + return ctrl.Result{}, err + } + + scope := shareScope{ + cluster: cluster, + managementCluster: managementCluster, + accountID: accountID, + ramClient: ramClient, + } // We need to share the transit gateway separately from the prefix list, as // the networktopology reconciler needs to attach the transit gateway // first, before moving on to creating the prefix list. If the transit // gateway isn't shared it won't be visible in the WC's account result := ctrl.Result{} - requeue, err := r.shareTransitGateway(ctx, cluster, managementCluster, accountID) + requeue, err := r.shareTransitGateway(ctx, scope) if err != nil { return ctrl.Result{}, err } @@ -134,7 +186,7 @@ func (r *ShareReconciler) reconcileNormal(ctx context.Context, cluster *capa.AWS result.RequeueAfter = ResourceMissingRequeDuration } - requeue, err = r.sharePrefixList(ctx, cluster, managementCluster, accountID) + requeue, err = r.sharePrefixList(ctx, scope) if err != nil { return ctrl.Result{}, err } @@ -171,10 +223,10 @@ func getPrefixListResourceShareName(cluster *capa.AWSCluster) string { return fmt.Sprintf("%s-%s", cluster.Name, "prefix-list") } -func (r *ShareReconciler) shareTransitGateway(ctx context.Context, cluster, managementCluster *capa.AWSCluster, accountID string) (requeue bool, err error) { +func (r *ShareReconciler) shareTransitGateway(ctx context.Context, scope shareScope) (requeue bool, err error) { logger := log.FromContext(ctx) - transitGatewayARN := getTransitGatewayARN(cluster, managementCluster) + transitGatewayARN := getTransitGatewayARN(scope.cluster, scope.managementCluster) if transitGatewayARN == "" { logger.Info("transit gateway arn annotation not set yet") @@ -183,18 +235,18 @@ func (r *ShareReconciler) shareTransitGateway(ctx context.Context, cluster, mana logger = logger.WithValues("transit-gateway-annotation", transitGatewayARN) - err = r.clusterClient.AddFinalizer(ctx, cluster, FinalizerResourceShare) + err = r.clusterClient.AddFinalizer(ctx, scope.cluster, FinalizerResourceShare) if err != nil { logger.Error(err, "failed to add finalizer") return false, err } - err = r.ramClient.ApplyResourceShare(ctx, resolver.ResourceShare{ - Name: getTransitGatewayResourceShareName(cluster), + err = scope.ramClient.ApplyResourceShare(ctx, resolver.ResourceShare{ + Name: getTransitGatewayResourceShareName(scope.cluster), ResourceArns: []string{ transitGatewayARN, }, - ExternalAccountID: accountID, + ExternalAccountID: scope.accountID, }) if err != nil { logger.Error(err, "failed to apply resource share") @@ -204,9 +256,9 @@ func (r *ShareReconciler) shareTransitGateway(ctx context.Context, cluster, mana return false, nil } -func (r *ShareReconciler) sharePrefixList(ctx context.Context, cluster, managementCluster *capa.AWSCluster, accountID string) (requeue bool, err error) { +func (r *ShareReconciler) sharePrefixList(ctx context.Context, scope shareScope) (requeue bool, err error) { logger := log.FromContext(ctx) - prefixListARN := getPrefixListARN(cluster, managementCluster) + prefixListARN := getPrefixListARN(scope.cluster, scope.managementCluster) if prefixListARN == "" { logger.Info("prefix list arn annotation not set yet") @@ -215,12 +267,12 @@ func (r *ShareReconciler) sharePrefixList(ctx context.Context, cluster, manageme logger = logger.WithValues("prefix-list-annotation", prefixListARN) - err = r.ramClient.ApplyResourceShare(ctx, resolver.ResourceShare{ - Name: getPrefixListResourceShareName(cluster), + err = scope.ramClient.ApplyResourceShare(ctx, resolver.ResourceShare{ + Name: getPrefixListResourceShareName(scope.cluster), ResourceArns: []string{ prefixListARN, }, - ExternalAccountID: accountID, + ExternalAccountID: scope.accountID, }) if err != nil { logger.Error(err, "failed to apply resource share") diff --git a/controllers/share_test.go b/controllers/share_test.go index 2a6a435f..20fe1d88 100644 --- a/controllers/share_test.go +++ b/controllers/share_test.go @@ -20,6 +20,7 @@ import ( "github.com/aws-resolver-rules-operator/controllers" "github.com/aws-resolver-rules-operator/controllers/controllersfakes" "github.com/aws-resolver-rules-operator/pkg/k8sclient" + "github.com/aws-resolver-rules-operator/pkg/resolver" "github.com/aws-resolver-rules-operator/pkg/resolver/resolverfakes" ) @@ -54,7 +55,7 @@ var _ = Describe("Share", func() { err := k8sClient.Patch(context.Background(), patchedIdentity, client.MergeFrom(identity)) Expect(err).NotTo(HaveOccurred()) - managementCluster = createRandomCluster( + _, managementCluster = createRandomClusterWithIdentity( annotation.NetworkTopologyModeAnnotation, annotation.NetworkTopologyModeGiantSwarmManaged, annotation.NetworkTopologyTransitGatewayIDAnnotation, @@ -71,11 +72,14 @@ var _ = Describe("Share", func() { } ramClient = new(resolverfakes.FakeRAMClient) + clientsFactory := &resolver.FakeClients{ + RAMClient: ramClient, + } clusterClient := k8sclient.NewAWSClusterClient(k8sClient) reconciler = controllers.NewShareReconciler( client.ObjectKeyFromObject(managementCluster), clusterClient, - ramClient, + clientsFactory, ) }) @@ -300,7 +304,7 @@ var _ = Describe("Share", func() { reconciler = controllers.NewShareReconciler( client.ObjectKeyFromObject(managementCluster), fakeClusterClient, - ramClient, + &resolver.FakeClients{}, ) }) diff --git a/main.go b/main.go index a4ef8a83..fbe0018f 100644 --- a/main.go +++ b/main.go @@ -219,7 +219,12 @@ func wireNetworkTopologyReconcilers(cfg reconcilerConfig, mgr manager.Manager) { Name: cfg.managementClusterName, } - if err := (controllers.NewRouteReconciler(managementCluster, cfg.clusterClient, cfg.awsClients)).SetupWithManager(mgr); err != nil { + routeReconciler := controllers.NewRouteReconciler( + managementCluster, + cfg.clusterClient, + cfg.awsClients, + ) + if err := routeReconciler.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Route") os.Exit(1) } @@ -229,9 +234,8 @@ func wireNetworkTopologyReconcilers(cfg reconcilerConfig, mgr manager.Manager) { cfg.awsClusterClient, cfg.awsClients, ) - if err := mcTransitGatewayReconciler.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller") + setupLog.Error(err, "unable to create controller", "controller", "ManagementClusterTransitGateway") os.Exit(1) } @@ -240,9 +244,8 @@ func wireNetworkTopologyReconcilers(cfg reconcilerConfig, mgr manager.Manager) { cfg.awsClusterClient, cfg.awsClients, ) - if err := tgwAttachmentReconciler.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller") + setupLog.Error(err, "unable to create controller", "controller", "TransitGatewayAttachment") os.Exit(1) } @@ -251,9 +254,18 @@ func wireNetworkTopologyReconcilers(cfg reconcilerConfig, mgr manager.Manager) { cfg.awsClusterClient, cfg.awsClients, ) - if err := prefixListEntryReconciler.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller") + setupLog.Error(err, "unable to create controller", "controller", "PrefixListEntry") + os.Exit(1) + } + + shareReconciler := controllers.NewShareReconciler( + managementCluster, + cfg.awsClusterClient, + cfg.awsClients, + ) + if err := shareReconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "Share") os.Exit(1) }