Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Zijun Wang committed Sep 27, 2023
1 parent 32f6daf commit 9704786
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 59 deletions.
105 changes: 50 additions & 55 deletions pkg/deploy/lattice/service_network_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import (

"github.com/golang/glog"

"github.com/aws/aws-application-networking-k8s/pkg/aws/services"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/vpclattice"

"github.com/aws/aws-application-networking-k8s/pkg/aws/services"
"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"

lattice_aws "github.com/aws/aws-application-networking-k8s/pkg/aws"
"github.com/aws/aws-application-networking-k8s/pkg/config"
latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
Expand Down Expand Up @@ -57,72 +58,72 @@ type defaultServiceNetworkManager struct {
// return errors.New(LATTICE_RETRY) when:
//
// CreateServiceNetworkVpcAssociationInput returns ServiceNetworkVpcAssociationStatusFailed/ServiceNetworkVpcAssociationStatusCreateInProgress/MeshVpcAssociationStatusDeleteInProgress
func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, stackSn *latticemodel.ServiceNetwork) (latticemodel.ServiceNetworkStatus, error) {
func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, desiredSN *latticemodel.ServiceNetwork) (latticemodel.ServiceNetworkStatus, error) {
// check if exists
service_networkSummary, err := m.cloud.Lattice().FindServiceNetwork(ctx, stackSn.Spec.Name, "")
serviceNetworkSummary, err := m.cloud.Lattice().FindServiceNetwork(ctx, desiredSN.Spec.Name, "")
if err != nil && !services.IsNotFoundError(err) {
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, err
}

// pre declaration
var service_networkID string
var service_networkArn string
var isServiceNetworkAlreadyAssociatedWithCurrentVpc bool
var isSnAlreadyAssociatedWithCurrentVpc bool
var snvaAssociatedWithCurrentVPC *vpclattice.ServiceNetworkVpcAssociationSummary
vpcLatticeSess := m.cloud.Lattice()
if service_networkSummary == nil {
glog.V(2).Infof("CreateOrUpdate ServiceNetwork, stackSn[%v] and tag it with vpciID[%s]\n", stackSn, config.VpcID)
if serviceNetworkSummary == nil {
glog.V(2).Infof("CreateOrUpdate ServiceNetwork, desiredSN[%v] and tag it with vpciID[%s]\n", desiredSN, config.VpcID)
// Add tag to show this is the VPC create this servicenetwork
// This means, the servicenetwork can only be deleted by the controller running in this VPC

serviceNetworkInput := vpclattice.CreateServiceNetworkInput{
Name: &stackSn.Spec.Name,
Name: &desiredSN.Spec.Name,
Tags: make(map[string]*string),
}
serviceNetworkInput.Tags[latticemodel.K8SServiceNetworkOwnedByVPC] = &config.VpcID

glog.V(2).Infof("CreateOrUpdate stackSn >>>> req[%v]", serviceNetworkInput)
glog.V(2).Infof("CreateOrUpdate CreateServiceNetworkInput: [%v]", serviceNetworkInput)
resp, err := vpcLatticeSess.CreateServiceNetworkWithContext(ctx, &serviceNetworkInput)
glog.V(2).Infof("CreateOrUpdate stackSn >>>> resp[%v], err : %v", resp, err)
glog.V(2).Infof("CreateOrUpdate CreateServiceNetworkOutput: [%v], err : %v", resp, err)
if err != nil {
glog.V(2).Infof("Failed to create stackSn[%v], err: %v", stackSn, err)
glog.V(2).Infof("Failed to create desiredSN[%v], err: %v", desiredSN, err)
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, err
}
service_networkID = aws.StringValue(resp.Id)
service_networkArn = aws.StringValue(resp.Arn)
isServiceNetworkAlreadyAssociatedWithCurrentVpc = false
isSnAlreadyAssociatedWithCurrentVpc = false
glog.V(6).Infof(" ServiceNetwork CreateOrUpdate API resp [%v]\n", resp)

} else { //service_networkSummary != nil
glog.V(6).Infof("stackSn[%v] exists, further check association", stackSn)
service_networkID = aws.StringValue(service_networkSummary.SvcNetwork.Id)
service_networkArn = aws.StringValue(service_networkSummary.SvcNetwork.Arn)
isServiceNetworkAlreadyAssociatedWithCurrentVpc, snvaAssociatedWithCurrentVPC, _, err = m.isServiceNetworkAlreadyAssociatedWithVPC(ctx, service_networkID)
} else {
glog.V(6).Infof("desiredSN[%v] exists, further check association", desiredSN)
service_networkID = aws.StringValue(serviceNetworkSummary.SvcNetwork.Id)
service_networkArn = aws.StringValue(serviceNetworkSummary.SvcNetwork.Arn)
isSnAlreadyAssociatedWithCurrentVpc, snvaAssociatedWithCurrentVPC, _, err = m.isServiceNetworkAlreadyAssociatedWithVPC(ctx, service_networkID)
if err != nil {
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, err
}
if stackSn.Spec.AssociateToVPC == true && isServiceNetworkAlreadyAssociatedWithCurrentVpc == true &&
if desiredSN.Spec.AssociateToVPC == true && isSnAlreadyAssociatedWithCurrentVpc == true &&
snvaAssociatedWithCurrentVPC.Status != nil && aws.StringValue(snvaAssociatedWithCurrentVPC.Status) == vpclattice.ServiceNetworkVpcAssociationStatusActive {
return m.UpdateServiceNetworkVpcAssociationIfNeeded(ctx, service_networkID, service_networkArn, stackSn, snvaAssociatedWithCurrentVPC.Id)
return m.UpdateServiceNetworkVpcAssociationIfNeeded(ctx, &serviceNetworkSummary.SvcNetwork, desiredSN, snvaAssociatedWithCurrentVPC.Id)

}
}

if stackSn.Spec.AssociateToVPC == true {
if isServiceNetworkAlreadyAssociatedWithCurrentVpc == false {
if desiredSN.Spec.AssociateToVPC == true {
if isSnAlreadyAssociatedWithCurrentVpc == false {
// current state: service network is associated to VPC
// desired state: associate this service network to VPC
createServiceNetworkVpcAssociationInput := vpclattice.CreateServiceNetworkVpcAssociationInput{
ServiceNetworkIdentifier: &service_networkID,
VpcIdentifier: &config.VpcID,
SecurityGroupIds: stackSn.Spec.SecurityGroupIds,
SecurityGroupIds: desiredSN.Spec.SecurityGroupIds,
}
glog.V(2).Infof("CreateOrUpdate stackSn/vpc association >>>> req[%v]", createServiceNetworkVpcAssociationInput)
glog.V(2).Infof("CreateOrUpdate desiredSN/vpc association >>>> req[%v]", createServiceNetworkVpcAssociationInput)
resp, err := vpcLatticeSess.CreateServiceNetworkVpcAssociationWithContext(ctx, &createServiceNetworkVpcAssociationInput)
glog.V(2).Infof("CreateOrUpdate stackSn and vpc association here >>>> resp[%v] err [%v]\n", resp, err)
// Associate stackSn with vpc
glog.V(2).Infof("CreateOrUpdate desiredSN and vpc association here >>>> resp[%v] err [%v]\n", resp, err)
// Associate desiredSN with vpc
if err != nil {
glog.V(2).Infof("Failed to associate stackSn[%v] and vpc, err: %v", stackSn, err)
glog.V(2).Infof("Failed to associate desiredSN[%v] and vpc, err: %v", desiredSN, err)
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, err
} else {
service_networkVPCAssociationStatus := aws.StringValue(resp.Status)
Expand All @@ -141,28 +142,28 @@ func (m *defaultServiceNetworkManager) CreateOrUpdate(ctx context.Context, stack
}
}
} else {
// stackSn.Spec.AssociateToVPC == false
if isServiceNetworkAlreadyAssociatedWithCurrentVpc == true {
// desiredSN.Spec.AssociateToVPC == false
if isSnAlreadyAssociatedWithCurrentVpc == true {
// current state: service network is associated to VPC
// desired state: not to associate this service network to VPC
glog.V(6).Infof("Disassociate stackSn(%v) from vpc association", stackSn.Spec.Name)
glog.V(6).Infof("Disassociate desiredSN(%v) from vpc association", desiredSN.Spec.Name)

deleteServiceNetworkVpcAssociationInput := vpclattice.DeleteServiceNetworkVpcAssociationInput{
ServiceNetworkVpcAssociationIdentifier: snvaAssociatedWithCurrentVPC.Id,
}

glog.V(2).Infof("Delete stackSn association >>>> req[%v]", deleteServiceNetworkVpcAssociationInput)
glog.V(2).Infof("Delete desiredSN association >>>> req[%v]", deleteServiceNetworkVpcAssociationInput)
resp, err := vpcLatticeSess.DeleteServiceNetworkVpcAssociationWithContext(ctx, &deleteServiceNetworkVpcAssociationInput)
glog.V(2).Infof("Delete stackSn association >>>> resp[%v],err [%v]", resp, err)
glog.V(2).Infof("Delete desiredSN association >>>> resp[%v],err [%v]", resp, err)
if err != nil {
glog.V(2).Infof("Failed to delete association for %v err=%v , resp = %v\n", stackSn.Spec.Name, err, resp)
glog.V(2).Infof("Failed to delete association for %v err=%v , resp = %v\n", desiredSN.Spec.Name, err, resp)
}

// return retry and check later if disassociation workflow finishes
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, errors.New(LATTICE_RETRY)

}
glog.V(2).Infof("Created stackSn(%v) without vpc association", stackSn.Spec.Name)
glog.V(2).Infof("Created desiredSN(%v) without vpc association", desiredSN.Spec.Name)
}
return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: service_networkArn, ServiceNetworkID: service_networkID}, nil
}
Expand Down Expand Up @@ -305,7 +306,7 @@ func (m *defaultServiceNetworkManager) isServiceNetworkAlreadyAssociatedWithVPC(
return false, nil, resp, err
}

func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociationIfNeeded(ctx context.Context, snId string, snArn string, desiredSN *latticemodel.ServiceNetwork, existingSnvaId *string) (latticemodel.ServiceNetworkStatus, error) {
func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociationIfNeeded(ctx context.Context, existingSN *vpclattice.ServiceNetworkSummary, desiredSN *latticemodel.ServiceNetwork, existingSnvaId *string) (latticemodel.ServiceNetworkStatus, error) {
retrievedSnva, err := m.cloud.Lattice().GetServiceNetworkVpcAssociationWithContext(ctx, &vpclattice.GetServiceNetworkVpcAssociationInput{
ServiceNetworkVpcAssociationIdentifier: existingSnvaId,
})
Expand All @@ -314,26 +315,23 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociationIfNeede
}

if len(desiredSN.Spec.SecurityGroupIds) == 0 && retrievedSnva.SecurityGroupIds != nil && len(retrievedSnva.SecurityGroupIds) > 0 {
// VPC latiice API have a limitation that the user can't remove all security groups if current snva already has security groups.
// 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
gwlog.FallbackLogger.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{
ServiceNetworkID: snId,
ServiceNetworkARN: snArn,
SnvaSecurityGroupIds: nil,
}, errors.New(LATTICE_RETRY)
return latticemodel.ServiceNetworkStatus{}, errors.New(LATTICE_RETRY)
}

twoSlicesElementsEqual := utils.PointerStringSlicesElementsEqual(desiredSN.Spec.SecurityGroupIds, retrievedSnva.SecurityGroupIds)

if twoSlicesElementsEqual {
// desiredSN's security group ids are same with retrievedSnva's security group ids, don't need to update
return latticemodel.ServiceNetworkStatus{
ServiceNetworkID: snId,
ServiceNetworkARN: snArn,
ServiceNetworkID: *existingSN.Id,
ServiceNetworkARN: *existingSN.Arn,
SnvaSecurityGroupIds: retrievedSnva.SecurityGroupIds,
}, nil
}
Expand All @@ -342,21 +340,18 @@ func (m *defaultServiceNetworkManager) UpdateServiceNetworkVpcAssociationIfNeede
ServiceNetworkVpcAssociationIdentifier: existingSnvaId,
SecurityGroupIds: desiredSN.Spec.SecurityGroupIds,
})
retSnStatus := latticemodel.ServiceNetworkStatus{
ServiceNetworkID: snId,
ServiceNetworkARN: snArn,
SnvaSecurityGroupIds: updateSnvaResp.SecurityGroupIds,
}
if err != nil {
return retSnStatus, err
return latticemodel.ServiceNetworkStatus{}, err
}
switch updateSnvaResp.Status {
case aws.String(vpclattice.ServiceNetworkVpcAssociationStatusActive):
return retSnStatus, nil
case aws.String(vpclattice.ServiceNetworkVpcAssociationStatusUpdateInProgress):
default:
return retSnStatus, errors.New(LATTICE_RETRY)

if *updateSnvaResp.Status == vpclattice.ServiceNetworkVpcAssociationStatusActive {
return latticemodel.ServiceNetworkStatus{
ServiceNetworkID: *existingSN.Id,
ServiceNetworkARN: *existingSN.Arn,
SnvaSecurityGroupIds: updateSnvaResp.SecurityGroupIds,
}, nil
} else {
return latticemodel.ServiceNetworkStatus{}, errors.New(LATTICE_RETRY)
}
return latticemodel.ServiceNetworkStatus{}, errors.New(LATTICE_RETRY)

}
8 changes: 4 additions & 4 deletions pkg/deploy/lattice/service_network_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,14 @@ func Test_defaultServiceNetworkManager_CreateOrUpdate_SnExists_SnvaExists_Update
Arn: &meshArn,
Id: &meshId,
SecurityGroupIds: securityGroupIds,
Status: aws.String(vpclattice.ServiceNetworkVpcAssociationStatusUpdateInProgress),
Status: aws.String(vpclattice.ServiceNetworkVpcAssociationStatusActive),
}, nil)

mockCloud.EXPECT().Lattice().Return(mockLattice).AnyTimes()
meshManager := NewDefaultServiceNetworkManager(mockCloud)
resp, err := meshManager.CreateOrUpdate(ctx, &meshCreateInput)

assert.Equal(t, err, errors.New(LATTICE_RETRY))
assert.Equal(t, err, nil)
assert.Equal(t, resp.ServiceNetworkARN, meshArn)
assert.Equal(t, resp.ServiceNetworkID, meshId)
assert.Equal(t, resp.SnvaSecurityGroupIds, securityGroupIds)
Expand Down Expand Up @@ -543,8 +543,8 @@ func Test_defaultServiceNetworkManager_CreateOrUpdate_SnExists_SnvaExists_Cannot
resp, err := meshManager.CreateOrUpdate(ctx, &desiredSn)

assert.Equal(t, err, errors.New(LATTICE_RETRY))
assert.Equal(t, resp.ServiceNetworkARN, meshArn)
assert.Equal(t, resp.ServiceNetworkID, meshId)
assert.Equal(t, resp.ServiceNetworkARN, "")
assert.Equal(t, resp.ServiceNetworkID, "")
}

func Test_CreateOrUpdateServiceNetwork_MeshAlreadyExist_AssociateToNotAssociate(t *testing.T) {
Expand Down

0 comments on commit 9704786

Please sign in to comment.