diff --git a/pkg/deploy/lattice/service_network_manager.go b/pkg/deploy/lattice/service_network_manager.go index e6f74997..dc3d7dbd 100644 --- a/pkg/deploy/lattice/service_network_manager.go +++ b/pkg/deploy/lattice/service_network_manager.go @@ -3,6 +3,7 @@ package lattice import ( "context" "errors" + "fmt" "golang.org/x/exp/slices" @@ -83,7 +84,7 @@ func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, desir } serviceNetworkInput.Tags[latticemodel.K8SServiceNetworkOwnedByVPC] = &config.VpcID - m.log.Debugf("Creating desiredSn %+v", serviceNetworkInput) + m.log.Debugf("Creating ServiceNetwork %+v", serviceNetworkInput) resp, err := vpcLatticeSess.CreateServiceNetworkWithContext(ctx, &serviceNetworkInput) if err != nil { return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, err @@ -92,7 +93,7 @@ func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, desir serviceNetworkId = aws.StringValue(resp.Id) serviceNetworkArn = aws.StringValue(resp.Arn) } else { - m.log.Debugf("desiredSn %s exists, checking its VPC association", desiredSn.Spec.Name) + m.log.Debugf("ServiceNetwork %s exists, checking its VPC association", desiredSn.Spec.Name) serviceNetworkId = aws.StringValue(foundSnSummary.SvcNetwork.Id) serviceNetworkArn = aws.StringValue(foundSnSummary.SvcNetwork.Arn) isSnAlreadyAssociatedWithCurrentVpc, snvaAssociatedWithCurrentVPC, _, err = m.isServiceNetworkAlreadyAssociatedWithVPC(ctx, serviceNetworkId) @@ -152,7 +153,7 @@ func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, desir // return retry and check later if disassociation workflow finishes return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, errors.New(LATTICE_RETRY) } - m.log.Debugf("Created desiredSn %s without VPC association", desiredSn.Spec.Name) + m.log.Debugf("Created ServiceNetwork %s without VPC association", desiredSn.Spec.Name) } return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: serviceNetworkArn, ServiceNetworkID: serviceNetworkId}, nil } @@ -294,20 +295,7 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociation(ctx co if err != nil { return latticemodel.ServiceNetworkStatus{}, err } - - if len(desiredSN.Spec.SecurityGroupIds) == 0 && retrievedSnva.SecurityGroupIds != nil && len(retrievedSnva.SecurityGroupIds) > 0 { - // VPC lattice API have a limitation that the user can't remove all security groups if current snva already has security groups. - // If that case happen, the controller will not delete snva. User should manually delete it. - //https://docs.aws.amazon.com/vpc-lattice/latest/ug/security-groups.html#security-groups-rules - m.log.Errorf("Can't remove all security groups from service network vpc association, for service network %s, snvaId %s", desiredSN.Spec.Name, retrievedSnva.Id) - - // TODO: Add validation webhook or update gateway (or VpcAssociationPolicy) status to indicate that error. - // https://gateway-api.sigs.k8s.io/geps/gep-713/#standard-status-condition - return latticemodel.ServiceNetworkStatus{}, errors.New(LATTICE_RETRY) - } - sgIdsEqual := securityGroupIdsEqual(desiredSN.Spec.SecurityGroupIds, retrievedSnva.SecurityGroupIds) - if sgIdsEqual { // desiredSN's security group ids are same with retrievedSnva's security group ids, don't need to update return latticemodel.ServiceNetworkStatus{ @@ -316,7 +304,6 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociation(ctx co SnvaSecurityGroupIds: retrievedSnva.SecurityGroupIds, }, nil } - updateSnvaResp, err := m.cloud.Lattice().UpdateServiceNetworkVpcAssociationWithContext(ctx, &vpclattice.UpdateServiceNetworkVpcAssociationInput{ ServiceNetworkVpcAssociationIdentifier: existingSnvaId, SecurityGroupIds: desiredSN.Spec.SecurityGroupIds, @@ -324,7 +311,6 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociation(ctx co if err != nil { return latticemodel.ServiceNetworkStatus{}, err } - if *updateSnvaResp.Status == vpclattice.ServiceNetworkVpcAssociationStatusActive { return latticemodel.ServiceNetworkStatus{ ServiceNetworkID: *existingSN.Id, @@ -332,9 +318,8 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociation(ctx co SnvaSecurityGroupIds: updateSnvaResp.SecurityGroupIds, }, nil } else { - return latticemodel.ServiceNetworkStatus{}, errors.New(LATTICE_RETRY) + return latticemodel.ServiceNetworkStatus{}, fmt.Errorf("%w, update svna status: %s", RetryErr, *updateSnvaResp.Status) } - } func securityGroupIdsEqual(arr1, arr2 []*string) bool { diff --git a/pkg/gateway/model_build_servicenetwork.go b/pkg/gateway/model_build_servicenetwork.go index d4925561..242201ea 100644 --- a/pkg/gateway/model_build_servicenetwork.go +++ b/pkg/gateway/model_build_servicenetwork.go @@ -85,10 +85,7 @@ func (t *serviceNetworkModelBuildTask) buildServiceNetwork(ctx context.Context) if t.vpcAssociationPolicy.Spec.AssociateWithVpc != nil { associateToVPC = *t.vpcAssociationPolicy.Spec.AssociateWithVpc } - - if t.vpcAssociationPolicy.Spec.SecurityGroupIds != nil { - spec.SecurityGroupIds = securityGroupIdsToStringPointersSlice(t.vpcAssociationPolicy.Spec.SecurityGroupIds) - } + spec.SecurityGroupIds = securityGroupIdsToStringPointersSlice(t.vpcAssociationPolicy.Spec.SecurityGroupIds) } spec.AssociateToVPC = associateToVPC defaultSN, err := config.GetClusterLocalGateway() @@ -118,7 +115,7 @@ type serviceNetworkModelBuildTask struct { } func securityGroupIdsToStringPointersSlice(sgIds []v1alpha1.SecurityGroupId) []*string { - var ret []*string + ret := make([]*string, 0) for _, sgId := range sgIds { sgIdStr := string(sgId) ret = append(ret, &sgIdStr)