Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Shawn Kaplan committed Oct 18, 2023
1 parent 870083f commit af52f91
Show file tree
Hide file tree
Showing 10 changed files with 500 additions and 516 deletions.
6 changes: 3 additions & 3 deletions controllers/accesslogpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an
return err
}

err = r.updateAccessLogPolicyAnnotations(ctx, alp, *stack)
err = r.updateAccessLogPolicyAnnotations(ctx, alp, stack)
if err != nil {
return err
}
Expand Down Expand Up @@ -238,7 +238,7 @@ func (r *accessLogPolicyReconciler) targetRefExists(ctx context.Context, alp *an
func (r *accessLogPolicyReconciler) buildAndDeployModel(
ctx context.Context,
alp *anv1alpha1.AccessLogPolicy,
) (*core.Stack, error) {
) (core.Stack, error) {
stack, _, err := r.modelBuilder.Build(ctx, alp)
if err != nil {
r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.AccessLogPolicyEventReasonFailedBuildModel,
Expand All @@ -257,7 +257,7 @@ func (r *accessLogPolicyReconciler) buildAndDeployModel(
}
r.log.Debugf("successfully deployed model for stack %s:%s", stack.StackID().Name, stack.StackID().Namespace)

return &stack, nil
return stack, nil
}

func (r *accessLogPolicyReconciler) updateAccessLogPolicyAnnotations(
Expand Down
10 changes: 10 additions & 0 deletions pkg/aws/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/vpclattice"
"golang.org/x/exp/maps"

"github.com/aws/aws-application-networking-k8s/pkg/aws/services"
"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
Expand All @@ -32,6 +33,9 @@ type Cloud interface {
// creates lattice tags with default values populated
DefaultTags() services.Tags

// creates lattice tags with default values populated and merges them with provided tags
DefaultTagsMergedWith(services.Tags) services.Tags

// check if tags map has managedBy tag
ContainsManagedBy(tags services.Tags) bool

Expand Down Expand Up @@ -97,6 +101,12 @@ func (c *defaultCloud) DefaultTags() services.Tags {
return tags
}

func (c *defaultCloud) DefaultTagsMergedWith(tags services.Tags) services.Tags {
newTags := c.DefaultTags()
maps.Copy(newTags, tags)
return newTags
}

func (c *defaultCloud) ContainsManagedBy(tags services.Tags) bool {
tag, ok := tags[TagManagedBy]
if !ok || tag == nil {
Expand Down
14 changes: 14 additions & 0 deletions pkg/aws/cloud_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions pkg/deploy/lattice/access_log_subscription_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ func (m *defaultAccessLogSubscriptionManager) Create(
return nil, fmt.Errorf("unsupported source type: %s", accessLogSubscription.Spec.SourceType)
}

tags := m.cloud.DefaultTags()
tags[lattice.AccessLogPolicyTagKey] = aws.String(accessLogSubscription.Spec.ALPNamespacedName.String())
tags := m.cloud.DefaultTagsMergedWith(services.Tags{
lattice.AccessLogPolicyTagKey: aws.String(accessLogSubscription.Spec.ALPNamespacedName.String()),
})

createALSInput := &vpclattice.CreateAccessLogSubscriptionInput{
ResourceIdentifier: &resourceIdentifier,
Expand Down
Loading

0 comments on commit af52f91

Please sign in to comment.