From a34b5eb36c346032d76d213ef7b07f253da101aa Mon Sep 17 00:00:00 2001 From: Scott Lai Date: Wed, 13 Sep 2023 13:15:21 -0700 Subject: [PATCH] fix: reset to default settings if healthCheck is removed (#369) --- pkg/deploy/lattice/target_group_manager.go | 94 +++++++- .../lattice/target_group_manager_test.go | 222 +++++++++++++++--- 2 files changed, 266 insertions(+), 50 deletions(-) diff --git a/pkg/deploy/lattice/target_group_manager.go b/pkg/deploy/lattice/target_group_manager.go index b95cf1ba..0bbd69c2 100644 --- a/pkg/deploy/lattice/target_group_manager.go +++ b/pkg/deploy/lattice/target_group_manager.go @@ -11,11 +11,12 @@ import ( "github.com/aws/aws-sdk-go/service/vpclattice" "fmt" + "strings" + lattice_aws "github.com/aws/aws-application-networking-k8s/pkg/aws" "github.com/aws/aws-application-networking-k8s/pkg/config" "github.com/aws/aws-application-networking-k8s/pkg/latticestore" latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" - "strings" ) type TargetGroupManager interface { @@ -73,22 +74,16 @@ func (s *defaultTargetGroupManager) Create(ctx context.Context, targetGroup *lat latticeTGName := getLatticeTGName(targetGroup) // check if exists tgSummary, err := s.findTargetGroup(ctx, targetGroup) + if err != nil { return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, err } vpcLatticeSess := s.cloud.Lattice() + + // this means Target Group already existed, so this is an update request if tgSummary != nil { - if targetGroup.Spec.Config.HealthCheckConfig != nil { - _, err := vpcLatticeSess.UpdateTargetGroupWithContext(ctx, &vpclattice.UpdateTargetGroupInput{ - HealthCheck: targetGroup.Spec.Config.HealthCheckConfig, - TargetGroupIdentifier: tgSummary.Id, - }) - if err != nil { - return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, err - } - } - return latticemodel.TargetGroupStatus{TargetGroupARN: aws.StringValue(tgSummary.Arn), TargetGroupID: aws.StringValue(tgSummary.Id)}, nil + return s.update(ctx, targetGroup, tgSummary) } glog.V(6).Infof("create targetgroup API here %v\n", targetGroup) @@ -171,6 +166,38 @@ func (s *defaultTargetGroupManager) Get(ctx context.Context, targetGroup *lattic return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, errors.New("Non existing Target Group") } +func (s *defaultTargetGroupManager) update(ctx context.Context, targetGroup *latticemodel.TargetGroup, tgSummary *vpclattice.TargetGroupSummary) (latticemodel.TargetGroupStatus, error) { + glog.V(6).Info("Target Group already existed. Updating instead") + + vpcLatticeSess := s.cloud.Lattice() + + healthCheckConfig := targetGroup.Spec.Config.HealthCheckConfig + + targetGroupStatus := latticemodel.TargetGroupStatus{ + TargetGroupARN: aws.StringValue(tgSummary.Arn), + TargetGroupID: aws.StringValue(tgSummary.Id), + } + + if healthCheckConfig == nil { + glog.V(6).Info("healthCheck is empty. Resetting to default settings") + + targetGroupProtocolVersion := targetGroup.Spec.Config.ProtocolVersion + + healthCheckConfig = s.getDefaultHealthCheckConfig(targetGroupProtocolVersion) + } + + _, err := vpcLatticeSess.UpdateTargetGroupWithContext(ctx, &vpclattice.UpdateTargetGroupInput{ + HealthCheck: healthCheckConfig, + TargetGroupIdentifier: tgSummary.Id, + }) + + if err != nil { + return latticemodel.TargetGroupStatus{}, err + } + + return targetGroupStatus, nil +} + func (s *defaultTargetGroupManager) Delete(ctx context.Context, targetGroup *latticemodel.TargetGroup) error { glog.V(6).Infof("Manager: Deleting target group %v \n", targetGroup) @@ -382,3 +409,48 @@ func (s *defaultTargetGroupManager) findTargetGroup(ctx context.Context, targetG } return nil, nil } + +// Get default health check configuration according to +// https://docs.aws.amazon.com/vpc-lattice/latest/ug/target-group-health-checks.html#health-check-settings +func (s *defaultTargetGroupManager) getDefaultHealthCheckConfig(targetGroupProtocolVersion string) *vpclattice.HealthCheckConfig { + var intResetValue int64 = 0 + + defaultMatcher := vpclattice.Matcher{ + HttpCode: aws.String("200"), + } + + defaultPath := "/" + + defaultProtocol := vpclattice.TargetGroupProtocolHttp + + if targetGroupProtocolVersion == "" { + targetGroupProtocolVersion = vpclattice.TargetGroupProtocolVersionHttp1 + } + + enabled := targetGroupProtocolVersion == vpclattice.TargetGroupProtocolVersionHttp1 + + healthCheckProtocolVersion := targetGroupProtocolVersion + + if targetGroupProtocolVersion == vpclattice.TargetGroupProtocolVersionGrpc { + healthCheckProtocolVersion = vpclattice.HealthCheckProtocolVersionHttp1 + } + + return &vpclattice.HealthCheckConfig{ + Enabled: &enabled, + + Protocol: &defaultProtocol, + ProtocolVersion: &healthCheckProtocolVersion, + + Path: &defaultPath, + Matcher: &defaultMatcher, + + // Set to nil to use the port of the targets + Port: nil, + + // Set to 0 to reset to default value + HealthCheckIntervalSeconds: &intResetValue, + HealthyThresholdCount: &intResetValue, + UnhealthyThresholdCount: &intResetValue, + HealthCheckTimeoutSeconds: &intResetValue, + } +} diff --git a/pkg/deploy/lattice/target_group_manager_test.go b/pkg/deploy/lattice/target_group_manager_test.go index dbec9382..5ecfa25e 100644 --- a/pkg/deploy/lattice/target_group_manager_test.go +++ b/pkg/deploy/lattice/target_group_manager_test.go @@ -3,17 +3,17 @@ package lattice import ( "context" "errors" + "reflect" "testing" - "github.com/aws/aws-application-networking-k8s/pkg/config" - "github.com/aws/aws-application-networking-k8s/pkg/model/core" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/service/vpclattice" - mocks_aws "github.com/aws/aws-application-networking-k8s/pkg/aws" mocks "github.com/aws/aws-application-networking-k8s/pkg/aws/services" + "github.com/aws/aws-application-networking-k8s/pkg/config" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/vpclattice" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) @@ -170,48 +170,83 @@ func Test_CreateTargetGroup_TGFailed_Active(t *testing.T) { } // target group status is active before creation, no need to recreate -func Test_CreateTargetGroup_TGActive_ACTIVE(t *testing.T) { - c := gomock.NewController(t) - defer c.Finish() - ctx := context.TODO() - tgSpec := latticemodel.TargetGroupSpec{ - Name: "test", - Config: latticemodel.TargetGroupConfig{ - Protocol: vpclattice.TargetGroupProtocolHttps, - ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1, - HealthCheckConfig: &vpclattice.HealthCheckConfig{ - Enabled: aws.Bool(true), +func Test_CreateTargetGroup_TGActive_UpdateHealthCheck(t *testing.T) { + tests := []struct { + healthCheckConfig *vpclattice.HealthCheckConfig + wantErr bool + }{ + { + healthCheckConfig: &vpclattice.HealthCheckConfig{ + Enabled: aws.Bool(false), }, + wantErr: false, + }, + { + healthCheckConfig: nil, + wantErr: false, + }, + { + wantErr: true, }, } - tgCreateInput := latticemodel.TargetGroup{ - ResourceMeta: core.ResourceMeta{}, - Spec: tgSpec, - } - mockVpcLatticeSess := mocks.NewMockLattice(c) + + ctx := context.TODO() + arn := "12345678912345678912" id := "12345678912345678912" - name := "test-https-http1" - beforeCreateStatus := vpclattice.TargetGroupStatusActive - tgSummary := vpclattice.TargetGroupSummary{ - Arn: &arn, - Id: &id, - Name: &name, - Status: &beforeCreateStatus, - } - listTgOutput := []*vpclattice.TargetGroupSummary{&tgSummary} + for _, test := range tests { + c := gomock.NewController(t) + defer c.Finish() - mockCloud := mocks_aws.NewMockCloud(c) - mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil) - mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, nil) - mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes() - tgManager := NewTargetGroupManager(mockCloud) - resp, err := tgManager.Create(ctx, &tgCreateInput) + mockVpcLatticeSess := mocks.NewMockLattice(c) + mockCloud := mocks_aws.NewMockCloud(c) + tgManager := NewTargetGroupManager(mockCloud) - assert.Nil(t, err) - assert.Equal(t, resp.TargetGroupARN, arn) - assert.Equal(t, resp.TargetGroupID, id) + tgSpec := latticemodel.TargetGroupSpec{ + Name: "test", + Config: latticemodel.TargetGroupConfig{ + Protocol: vpclattice.TargetGroupProtocolHttps, + ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1, + HealthCheckConfig: test.healthCheckConfig, + }, + } + + tgCreateInput := latticemodel.TargetGroup{ + ResourceMeta: core.ResourceMeta{}, + Spec: tgSpec, + } + + tgSummary := vpclattice.TargetGroupSummary{ + Arn: &arn, + Id: &id, + Name: aws.String("test-https-http1"), + Status: aws.String(vpclattice.TargetGroupStatusActive), + Port: aws.Int64(80), + } + + listTgOutput := []*vpclattice.TargetGroupSummary{&tgSummary} + + mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil) + + if test.wantErr { + mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, errors.New("error")) + } else { + mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, nil) + } + + mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes() + + resp, err := tgManager.Create(ctx, &tgCreateInput) + + if test.wantErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + assert.Equal(t, resp.TargetGroupARN, arn) + assert.Equal(t, resp.TargetGroupID, id) + } + } } // target group status is create-in-progress before creation, return Retry @@ -1105,3 +1140,112 @@ func Test_Get(t *testing.T) { } } } + +func Test_defaultTargetGroupManager_getDefaultHealthCheckConfig(t *testing.T) { + var ( + resetValue = aws.Int64(0) + defaultMatcher = &vpclattice.Matcher{ + HttpCode: aws.String("200"), + } + defaultPath = aws.String("/") + defaultProtocol = aws.String(vpclattice.TargetGroupProtocolHttp) + ) + + type args struct { + targetGroupProtocolVersion string + } + + tests := []struct { + name string + args args + want *vpclattice.HealthCheckConfig + }{ + { + name: "HTTP1 default health check config", + args: args{ + targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1, + }, + want: &vpclattice.HealthCheckConfig{ + Enabled: aws.Bool(true), + HealthCheckIntervalSeconds: resetValue, + HealthCheckTimeoutSeconds: resetValue, + HealthyThresholdCount: resetValue, + UnhealthyThresholdCount: resetValue, + Matcher: defaultMatcher, + Path: defaultPath, + Port: nil, + Protocol: defaultProtocol, + ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1), + }, + }, + { + name: "empty target group protocol version default health check config", + args: args{ + targetGroupProtocolVersion: "", + }, + want: &vpclattice.HealthCheckConfig{ + Enabled: aws.Bool(true), + HealthCheckIntervalSeconds: resetValue, + HealthCheckTimeoutSeconds: resetValue, + HealthyThresholdCount: resetValue, + UnhealthyThresholdCount: resetValue, + Matcher: defaultMatcher, + Path: defaultPath, + Port: nil, + Protocol: defaultProtocol, + ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1), + }, + }, + { + name: "HTTP2 default health check config", + args: args{ + targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp2, + }, + want: &vpclattice.HealthCheckConfig{ + Enabled: aws.Bool(false), + HealthCheckIntervalSeconds: resetValue, + HealthCheckTimeoutSeconds: resetValue, + HealthyThresholdCount: resetValue, + UnhealthyThresholdCount: resetValue, + Matcher: defaultMatcher, + Path: defaultPath, + Port: nil, + Protocol: defaultProtocol, + ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp2), + }, + }, + { + name: "GRPC default health check config", + args: args{ + targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionGrpc, + }, + want: &vpclattice.HealthCheckConfig{ + Enabled: aws.Bool(false), + HealthCheckIntervalSeconds: resetValue, + HealthCheckTimeoutSeconds: resetValue, + HealthyThresholdCount: resetValue, + UnhealthyThresholdCount: resetValue, + Matcher: defaultMatcher, + Path: defaultPath, + Port: nil, + Protocol: defaultProtocol, + ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := gomock.NewController(t) + defer c.Finish() + + mockCloud := mocks_aws.NewMockCloud(c) + + s := NewTargetGroupManager(mockCloud) + + if got := s.getDefaultHealthCheckConfig(tt.args.targetGroupProtocolVersion); !reflect.DeepEqual(got, tt.want) { + t.Errorf("defaultTargetGroupManager.getDefaultHealthCheckConfig() = %v, want %v", got, tt.want) + } + }) + } +}