Skip to content

Commit

Permalink
Enable share reconciler (#216)
Browse files Browse the repository at this point in the history
* Enable share reconciler

* update CHANGELOG

* Use management cluster account to add to prefix list entries

Shared prefix lists cannot be modified
  • Loading branch information
mnitchev authored Sep 30, 2024
1 parent b8a4d02 commit 88d480a
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 32 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion controllers/prefix_list_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions controllers/prefix_list_entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
90 changes: 71 additions & 19 deletions controllers/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,18 @@ const (
)

type ShareReconciler struct {
ramClient resolver.RAMClient
awsClients resolver.AWSClients
clusterClient AWSClusterClient
managementCluster k8stypes.NamespacedName
}

func NewShareReconciler(
managementCluster types.NamespacedName,
clusterClient AWSClusterClient,
ramClient resolver.RAMClient,
awsClients resolver.AWSClients,
) *ShareReconciler {
return &ShareReconciler{
ramClient: ramClient,
awsClients: awsClients,
clusterClient: clusterClient,
managementCluster: managementCluster,
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -118,23 +159,34 @@ 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
}
if requeue {
result.RequeueAfter = ResourceMissingRequeDuration
}

requeue, err = r.sharePrefixList(ctx, cluster, managementCluster, accountID)
requeue, err = r.sharePrefixList(ctx, scope)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
10 changes: 7 additions & 3 deletions controllers/share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand All @@ -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,
)
})

Expand Down Expand Up @@ -300,7 +304,7 @@ var _ = Describe("Share", func() {
reconciler = controllers.NewShareReconciler(
client.ObjectKeyFromObject(managementCluster),
fakeClusterClient,
ramClient,
&resolver.FakeClients{},
)
})

Expand Down
26 changes: 19 additions & 7 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand Down

0 comments on commit 88d480a

Please sign in to comment.