Skip to content
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
24 changes: 10 additions & 14 deletions pkg/deploy/lattice/listener_synthesizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,15 @@ func (l *listenerSynthesizer) Synthesize(ctx context.Context) error {
return err
}

// deletes are deferred to the later logic comparing existing listeners
// to current listeners.
if !listener.IsDeleted {
status, err := l.listenerMgr.Upsert(ctx, listener, svc)
if err != nil {
listenerErr = errors.Join(listenerErr,
fmt.Errorf("failed ListenerManager.Upsert %s-%s due to err %s",
listener.Spec.K8SRouteName, listener.Spec.K8SRouteNamespace, err))
continue
}

listener.Status = &status
status, err := l.listenerMgr.Upsert(ctx, listener, svc)
if err != nil {
listenerErr = errors.Join(listenerErr,
fmt.Errorf("failed ListenerManager.Upsert %s-%s due to err %s",
listener.Spec.K8SRouteName, listener.Spec.K8SRouteNamespace, err))
continue
}

listener.Status = &status
}

if listenerErr != nil {
Expand Down Expand Up @@ -86,8 +82,8 @@ func (l *listenerSynthesizer) Synthesize(ctx context.Context) error {
func (l *listenerSynthesizer) shouldDelete(listenerToFind *model.Listener, stackListeners []*model.Listener) bool {
for _, candidate := range stackListeners {
if candidate.Spec.Port == listenerToFind.Spec.Port && candidate.Spec.Protocol == listenerToFind.Spec.Protocol {
// found a match, delete if match is deleted
return candidate.IsDeleted
// found a match, do not delete
return false
}
}
// there is no matching listener
Expand Down
31 changes: 0 additions & 31 deletions pkg/deploy/lattice/listener_synthesizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,6 @@ func Test_SynthesizeListenerCreate(t *testing.T) {
assert.Nil(t, err)
}

func Test_SynthesizeListenerDeleteNoOp(t *testing.T) {
c := gomock.NewController(t)
defer c.Finish()
ctx := context.TODO()
mockListenerMgr := NewMockListenerManager(c)

stack := core.NewDefaultStack(core.StackID{Name: "foo", Namespace: "bar"})

svc := &model.Service{
ResourceMeta: core.NewResourceMeta(stack, "AWS:VPCServiceNetwork::Service", "stack-svc-id"),
Status: &model.ServiceStatus{Id: "svc-id"},
}
assert.NoError(t, stack.AddResource(svc))

l := &model.Listener{
ResourceMeta: core.NewResourceMeta(stack, "AWS:VPCServiceNetwork::Listener", "l-id"),
Spec: model.ListenerSpec{
StackServiceId: "stack-svc-id",
},
IsDeleted: true, // <-- the bit that matters
}
assert.NoError(t, stack.AddResource(l))

// since we aren't returning any resources, we interpret that to mean the listener is already deleted
mockListenerMgr.EXPECT().List(ctx, gomock.Any()).Return([]*vpclattice.ListenerSummary{}, nil)

ls := NewListenerSynthesizer(gwlog.FallbackLogger, mockListenerMgr, stack)
err := ls.Synthesize(ctx)
assert.Nil(t, err)
}

func Test_SynthesizeListenerCreateWithReconcile(t *testing.T) {
c := gomock.NewController(t)
defer c.Finish()
Expand Down
23 changes: 19 additions & 4 deletions pkg/deploy/lattice/rule_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,24 @@ func (r *defaultRuleManager) buildLatticeRule(modelRule *model.Rule) (*vpclattic
updateMatchFromRule(&httpMatch, modelRule)
gro.Match = &vpclattice.RuleMatch{HttpMatch: &httpMatch}

if len(modelRule.Spec.Action.TargetGroups) > 0 {
// check if we have at least one valid target group
var hasValidTargetGroup bool
for _, tg := range modelRule.Spec.Action.TargetGroups {
if tg.LatticeTgId != model.InvalidBackendRefTgId {
hasValidTargetGroup = true
break
}
}

if hasValidTargetGroup {
var latticeTGs []*vpclattice.WeightedTargetGroup
for _, ruleTg := range modelRule.Spec.Action.TargetGroups {
// skip any invalid TGs - eventually VPC Lattice may support weighted fixed response
// and this logic can be more in line with the spec
if ruleTg.LatticeTgId == model.InvalidBackendRefTgId {
continue
}

latticeTG := vpclattice.WeightedTargetGroup{
TargetGroupIdentifier: aws.String(ruleTg.LatticeTgId),
Weight: aws.Int64(ruleTg.Weight),
Expand All @@ -114,7 +129,7 @@ func (r *defaultRuleManager) buildLatticeRule(modelRule *model.Rule) (*vpclattic
},
}
} else {
r.log.Debugf("There are no target groups, defaulting to 404 Fixed response")
r.log.Debugf("There are no valid target groups, defaulting to 404 Fixed response")
gro.Action = &vpclattice.RuleAction{
FixedResponse: &vpclattice.FixedResponseAction{
StatusCode: aws.Int64(404),
Expand Down Expand Up @@ -237,7 +252,7 @@ func (r *defaultRuleManager) updateIfNeeded(

_, err := r.cloud.Lattice().UpdateRuleWithContext(ctx, &uri)
if err != nil {
return model.RuleStatus{}, fmt.Errorf("Failed UpdateRule %d for %s, %s due to %s",
return model.RuleStatus{}, fmt.Errorf("failed UpdateRule %d for %s, %s due to %s",
ruleToUpdate.Priority, latticeListenerId, latticeSvcId, err)
}

Expand Down Expand Up @@ -375,7 +390,7 @@ func (r *defaultRuleManager) Delete(ctx context.Context, ruleId string, serviceI

_, err := r.cloud.Lattice().DeleteRuleWithContext(ctx, &deleteInput)
if err != nil {
return fmt.Errorf("Failed DeleteRule %s/%s/%s due to %s", serviceId, listenerId, ruleId, err)
return fmt.Errorf("failed DeleteRule %s/%s/%s due to %s", serviceId, listenerId, ruleId, err)
}

r.log.Infof("Success DeleteRule %s/%s/%s", serviceId, listenerId, ruleId)
Expand Down
110 changes: 110 additions & 0 deletions pkg/deploy/lattice/rule_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,67 @@ func Test_Create(t *testing.T) {
},
}

rInvalidBR := &model.Rule{
Spec: model.RuleSpec{
Priority: 1,
Action: model.RuleAction{
TargetGroups: []*model.RuleTargetGroup{
{
LatticeTgId: model.InvalidBackendRefTgId,
Weight: 1,
},
},
},
PathMatchPrefix: true,
PathMatchValue: "/foo",
},
}

rTwoInvalidBR := &model.Rule{
Spec: model.RuleSpec{
Priority: 1,
Action: model.RuleAction{
TargetGroups: []*model.RuleTargetGroup{
{
LatticeTgId: model.InvalidBackendRefTgId,
Weight: 1,
},
{
LatticeTgId: model.InvalidBackendRefTgId,
Weight: 1,
},
},
},
PathMatchPrefix: true,
PathMatchValue: "/foo",
},
}

rOneValidBR := &model.Rule{
Spec: model.RuleSpec{
Priority: 1,
Method: "POST",
Action: model.RuleAction{
TargetGroups: []*model.RuleTargetGroup{
{
LatticeTgId: model.InvalidBackendRefTgId,
Weight: 1,
},
{
LatticeTgId: model.InvalidBackendRefTgId,
Weight: 1,
},
{
LatticeTgId: "tg-id",
Weight: 1,
},
},
},
PathMatchPrefix: true,
PathMatchValue: "/foo",
},
}

t.Run("test create", func(t *testing.T) {
mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return(
[]*vpclattice.GetRuleOutput{}, nil)
Expand Down Expand Up @@ -183,6 +244,55 @@ func Test_Create(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, "existing-arn", ruleStatus.Arn)
})

t.Run("test create - invalid backendRefs", func(t *testing.T) {
mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return(
[]*vpclattice.GetRuleOutput{}, nil).Times(2)

mockLattice.EXPECT().CreateRuleWithContext(ctx, gomock.Any()).DoAndReturn(
func(ctx context.Context, input *vpclattice.CreateRuleInput, i ...interface{}) (*vpclattice.CreateRuleOutput, error) {
assert.Equal(t, int64(404), aws.Int64Value(input.Action.FixedResponse.StatusCode))

return &vpclattice.CreateRuleOutput{
Arn: aws.String("arn"),
Id: aws.String("id"),
Name: aws.String("name"),
}, nil
}).Times(2)

rm := NewRuleManager(gwlog.FallbackLogger, cloud)
ruleStatus, err := rm.Upsert(ctx, rInvalidBR, l, svc)
assert.Nil(t, err)
assert.Equal(t, "arn", ruleStatus.Arn)

// result should be the same so long as all backendRefs are invalid
ruleStatus, err = rm.Upsert(ctx, rTwoInvalidBR, l, svc)
assert.Nil(t, err)
assert.Equal(t, "arn", ruleStatus.Arn)
})

t.Run("test create - one valid backendRef, two invalid", func(t *testing.T) {
mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return(
[]*vpclattice.GetRuleOutput{}, nil)

mockLattice.EXPECT().CreateRuleWithContext(ctx, gomock.Any()).DoAndReturn(
func(ctx context.Context, input *vpclattice.CreateRuleInput, i ...interface{}) (*vpclattice.CreateRuleOutput, error) {
assert.Equal(t, "POST", aws.StringValue(input.Match.HttpMatch.Method))
assert.Equal(t, 1, len(input.Action.Forward.TargetGroups))
assert.Equal(t, "tg-id", aws.StringValue(input.Action.Forward.TargetGroups[0].TargetGroupIdentifier))

return &vpclattice.CreateRuleOutput{
Arn: aws.String("arn"),
Id: aws.String("id"),
Name: aws.String("name"),
}, nil
})

rm := NewRuleManager(gwlog.FallbackLogger, cloud)
ruleStatus, err := rm.Upsert(ctx, rOneValidBR, l, svc)
assert.Nil(t, err)
assert.Equal(t, "arn", ruleStatus.Arn)
})
}

func Test_CreateWithTempPriority(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/deploy/lattice/rule_synthesizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ func (r *ruleSynthesizer) resolveRuleTgIds(ctx context.Context, modelRule *model
}

if rtg.StackTargetGroupId != "" {
if rtg.StackTargetGroupId == model.InvalidBackendRefTgId {
r.log.Debugf("Rule TG has an invalid backendref, setting TG id to invalid")
rtg.LatticeTgId = model.InvalidBackendRefTgId
continue
}

r.log.Debugf("Fetching TG %d from the stack (ID %s)", i, rtg.StackTargetGroupId)

stackTg := &model.TargetGroup{}
Expand Down
4 changes: 4 additions & 0 deletions pkg/deploy/lattice/rule_synthesizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ func Test_resolveRuleTgs(t *testing.T) {
{
StackTargetGroupId: "stack-tg-id",
},
{
StackTargetGroupId: model.InvalidBackendRefTgId,
},
},
},
},
Expand Down Expand Up @@ -197,4 +200,5 @@ func Test_resolveRuleTgs(t *testing.T) {

assert.Equal(t, "svc-export-tg-id", r.Spec.Action.TargetGroups[0].LatticeTgId)
assert.Equal(t, "tg-id", r.Spec.Action.TargetGroups[1].LatticeTgId)
assert.Equal(t, model.InvalidBackendRefTgId, r.Spec.Action.TargetGroups[2].LatticeTgId)
}
10 changes: 4 additions & 6 deletions pkg/gateway/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"errors"
"fmt"
model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
apierrors "k8s.io/apimachinery/pkg/api/errors"

"k8s.io/apimachinery/pkg/types"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
)
Expand All @@ -27,9 +25,6 @@ func (t *latticeServiceModelBuildTask) extractListenerInfo(
t.log.Debugf("Building Listener for Route %s-%s", t.route.Name(), t.route.Namespace())
gw, err := t.getGateway(ctx)
if err != nil {
if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() {
return 0, string(protocol), nil // ok if we're deleting the route
}
return 0, "", err
}

Expand Down Expand Up @@ -83,6 +78,10 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack
if len(t.route.Spec().ParentRefs()) == 0 {
t.log.Debugf("No ParentRefs on route %s-%s, nothing to do", t.route.Name(), t.route.Namespace())
}
if !t.route.DeletionTimestamp().IsZero() {
t.log.Debugf("Route %s-%s is deleted, skipping listener build", t.route.Name(), t.route.Namespace())
return nil
}

for _, parentRef := range t.route.Spec().ParentRefs() {
if parentRef.Name != t.route.Spec().ParentRefs()[0].Name {
Expand Down Expand Up @@ -112,7 +111,6 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack

t.log.Debugf("Added listener %s-%s to the stack (ID %s)",
modelListener.Spec.K8SRouteName, modelListener.Spec.K8SRouteNamespace, modelListener.ID())
modelListener.IsDeleted = !t.route.DeletionTimestamp().IsZero()
}

return nil
Expand Down
13 changes: 10 additions & 3 deletions pkg/gateway/model_build_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"

Expand All @@ -31,6 +30,7 @@ const (
)

func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackListenerId string) error {
// note we only build rules for non-deleted routes
t.log.Debugf("Processing %d rules", len(t.route.Spec().Rules()))

for i, rule := range t.route.Spec().Rules() {
Expand Down Expand Up @@ -254,9 +254,16 @@ func (t *latticeServiceModelBuildTask) getTargetGroupsForRuleAction(ctx context.
// generate the actual target group model for the backendRef
_, tg, err := t.brTgBuilder.Build(ctx, t.route, backendRef, t.stack)
if err != nil {
return nil, err
ibre := &InvalidBackendRefError{}
if !errors.As(err, &ibre) {
return nil, err
}

t.log.Infof("Invalid backendRef found on route %s", t.route.Name())
ruleTG.StackTargetGroupId = model.InvalidBackendRefTgId
} else {
ruleTG.StackTargetGroupId = tg.ID()
}
ruleTG.StackTargetGroupId = tg.ID()
}

tgList = append(tgList, &ruleTG)
Expand Down
Loading