Skip to content

Add Tagging to lattice target group resource #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 1, 2022
Merged

Add Tagging to lattice target group resource #59

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0dd8599
Add targetgroup taggings
liwenwu-amazon Dec 1, 2022
6857488
Add targetgroup tagging when it is created due to HTTProute
liwenwu-amazon Dec 1, 2022
bc90562
Add tg tagging unit test for serviceexport
liwenwu-amazon Dec 1, 2022
072c1d8
Add unit test for tagging tg created by HTTProute backendref
liwenwu-amazon Dec 1, 2022
3c8a0ad
Fixed the tg tagging unit test failure
liwenwu-amazon Dec 1, 2022
240182e
Address CR comments
liwenwu-amazon Dec 1, 2022
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pkg/deploy/lattice/target_group_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,20 @@ func (s *defaultTargetGroupManager) Create(ctx context.Context, targetGroup *lat
Config: config,
Name: &targetGroup.Spec.Name,
Type: &targetGroupType,
Tags: make(map[string]*string),
}
createTargetGroupInput.Tags[latticemodel.K8SServiceNameKey] = &targetGroup.Spec.Config.K8SServiceName
createTargetGroupInput.Tags[latticemodel.K8SServiceNamespaceKey] = &targetGroup.Spec.Config.K8SServiceNamespace
if targetGroup.Spec.Config.IsServiceExport {
value := latticemodel.K8SIsServiceExport
createTargetGroupInput.Tags[latticemodel.K8SIsServiceExportKey] = &value
} else {
value := latticemodel.K8SIsNotServiceExport
createTargetGroupInput.Tags[latticemodel.K8SIsServiceExportKey] = &value
Copy link

@M00nF1sh M00nF1sh Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i prefer string types instead of bool types for extensibility(in case targetGroup needed for other resources)
e.g. Tags[resourceType] = "serviceExport"
Copy link
Contributor

@ellistarn ellistarn Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further, you can use a library to pointer-ify these for you inline: lo.ToPtr(string) or aws.String(string)
createTargetGroupInput.Tags[latticemodel.K8SHTTPRouteNameKey] = &targetGroup.Spec.Config.K8SHTTPRouteName
createTargetGroupInput.Tags[latticemodel.K8SHTTPRouteNamespaceKey] = &targetGroup.Spec.Config.K8SHTTPRouteNamespace
}

vpcLatticeSess := s.cloud.Lattice()
resp, err := vpcLatticeSess.CreateTargetGroupWithContext(ctx, &createTargetGroupInput)
glog.V(2).Infof("create target group >>>> req [%v], resp[%v] err[%v]\n", createTargetGroupInput, resp, err)
Expand Down
140 changes: 91 additions & 49 deletions pkg/deploy/lattice/target_group_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,59 +21,101 @@ func Test_CreateTargetGroup_TGNotExist_Active(t *testing.T) {
defer c.Finish()
ctx := context.TODO()

tgSpec := latticemodel.TargetGroupSpec{
Name: "test",
Config: latticemodel.TargetGroupConfig{
Port: int32(8080),
Protocol: "HTTP",
VpcID: config.VpcID,
EKSClusterName: "",
IsServiceImport: false,
},
}
tgCreateInput := latticemodel.TargetGroup{
ResourceMeta: core.ResourceMeta{},
Spec: tgSpec,
}
mockVpcLatticeSess := mocks.NewMockLattice(c)
arn := "12345678912345678912"
id := "12345678912345678912"
name := "test"
tgStatus := vpclattice.TargetGroupStatusActive
tgCreateOutput := &vpclattice.CreateTargetGroupOutput{
Arn: &arn,
Id: &id,
Name: &name,
Status: &tgStatus,
}
p := int64(8080)
prot := "HTTP"
emptystring := ""
config := &vpclattice.TargetGroupConfig{
Port: &p,
Protocol: &prot,
VpcIdentifier: &config.VpcID,
ProtocolVersion: &emptystring,
}
tg_types := [2]string{"by-backendref", "by-serviceexport"}

for _, tg_type := range tg_types {
var tgSpec latticemodel.TargetGroupSpec

if tg_type == "by-serviceexport" {
// testing targetgroup for serviceexport
tgSpec = latticemodel.TargetGroupSpec{
Name: "test",
Config: latticemodel.TargetGroupConfig{
Port: int32(8080),
Protocol: "HTTP",
VpcID: config.VpcID,
EKSClusterName: "",
IsServiceImport: false,
IsServiceExport: true,
K8SServiceName: "exportsvc1",
K8SServiceNamespace: "default",
},
}
} else if tg_type == "by-backendref" {
// testing targetgroup for serviceexport
tgSpec = latticemodel.TargetGroupSpec{
Name: "test",
Config: latticemodel.TargetGroupConfig{
Port: int32(8080),
Protocol: "HTTP",
VpcID: config.VpcID,
EKSClusterName: "",
IsServiceImport: false,
IsServiceExport: false,
K8SServiceName: "backend-svc1",
K8SServiceNamespace: "default",
K8SHTTPRouteName: "httproute1",
K8SHTTPRouteNamespace: "default",
},
}
}
tgCreateInput := latticemodel.TargetGroup{
ResourceMeta: core.ResourceMeta{},
Spec: tgSpec,
}
mockVpcLatticeSess := mocks.NewMockLattice(c)
arn := "12345678912345678912"
id := "12345678912345678912"
name := "test"
tgStatus := vpclattice.TargetGroupStatusActive
tgCreateOutput := &vpclattice.CreateTargetGroupOutput{
Arn: &arn,
Id: &id,
Name: &name,
Status: &tgStatus,
}
p := int64(8080)
prot := "HTTP"
emptystring := ""
config := &vpclattice.TargetGroupConfig{
Port: &p,
Protocol: &prot,
VpcIdentifier: &config.VpcID,
ProtocolVersion: &emptystring,
}

createTargetGroupInput := vpclattice.CreateTargetGroupInput{
Config: config,
Name: &name,
Type: &emptystring,
}
createTargetGroupInput := vpclattice.CreateTargetGroupInput{
Config: config,
Name: &name,
Type: &emptystring,
Tags: make(map[string]*string),
}
createTargetGroupInput.Tags[latticemodel.K8SServiceNameKey] = &tgSpec.Config.K8SServiceName
createTargetGroupInput.Tags[latticemodel.K8SServiceNamespaceKey] = &tgSpec.Config.K8SServiceNamespace

if tg_type == "by-serviceexport" {
value := latticemodel.K8SIsServiceExport
createTargetGroupInput.Tags[latticemodel.K8SIsServiceExportKey] = &value
} else if tg_type == "by-backendref" {
value := latticemodel.K8SIsNotServiceExport
createTargetGroupInput.Tags[latticemodel.K8SIsServiceExportKey] = &value
createTargetGroupInput.Tags[latticemodel.K8SHTTPRouteNameKey] = &tgSpec.Config.K8SHTTPRouteName
createTargetGroupInput.Tags[latticemodel.K8SHTTPRouteNamespaceKey] = &tgSpec.Config.K8SHTTPRouteNamespace
}

listTgOutput := []*vpclattice.TargetGroupSummary{}
listTgOutput := []*vpclattice.TargetGroupSummary{}

mockCloud := mocks_aws.NewMockCloud(c)
mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil)
mockVpcLatticeSess.EXPECT().CreateTargetGroupWithContext(ctx, &createTargetGroupInput).Return(tgCreateOutput, nil)
mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes()
tgManager := NewTargetGroupManager(mockCloud)
resp, err := tgManager.Create(ctx, &tgCreateInput)
mockCloud := mocks_aws.NewMockCloud(c)
mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil)
mockVpcLatticeSess.EXPECT().CreateTargetGroupWithContext(ctx, &createTargetGroupInput).Return(tgCreateOutput, nil)
mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes()
tgManager := NewTargetGroupManager(mockCloud)
resp, err := tgManager.Create(ctx, &tgCreateInput)

assert.Nil(t, err)
assert.Equal(t, resp.TargetGroupARN, arn)
assert.Equal(t, resp.TargetGroupID, id)
assert.Nil(t, err)
assert.Equal(t, resp.TargetGroupARN, arn)
assert.Equal(t, resp.TargetGroupID, id)
}
}

// target group status is failed, and is active after creation
Expand Down
26 changes: 17 additions & 9 deletions pkg/gateway/model_build_targetgroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,12 @@ func (t *targetGroupModelBuildTask) BuildTargetGroup(ctx context.Context) error
Config: latticemodel.TargetGroupConfig{
VpcID: config.VpcID,
//Port: backendServicePort,
IsServiceImport: false,
Protocol: "HTTP",
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
IsServiceImport: false,
IsServiceExport: true,
K8SServiceName: t.serviceExport.Name,
K8SServiceNamespace: t.serviceExport.Namespace,
Protocol: "HTTP",
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
},
}

Expand Down Expand Up @@ -341,12 +344,17 @@ func (t *latticeServiceModelBuildTask) buildHTTPTargetGroupSpec(ctx context.Cont
Name: tgName,
Type: latticemodel.TargetGroupTypeIP,
Config: latticemodel.TargetGroupConfig{
VpcID: vpc,
EKSClusterName: ekscluster,
IsServiceImport: isServiceImport,
Protocol: "HTTP",
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
Port: backendServicePort,
VpcID: vpc,
EKSClusterName: ekscluster,
IsServiceImport: isServiceImport,
IsServiceExport: false,
K8SServiceName: string(httpBackendRef.Name),
K8SServiceNamespace: namespace,
K8SHTTPRouteName: t.httpRoute.Name,
K8SHTTPRouteNamespace: t.httpRoute.Namespace,
Protocol: "HTTP",
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
Port: backendServicePort,
},
IsDeleted: isDeleted,
}, nil
Expand Down
16 changes: 16 additions & 0 deletions pkg/model/lattice/targetgroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ import (
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
)

const (
K8SServiceNameKey = "K8SServiceName"
K8SServiceNamespaceKey = "K8SServiceNamespace"
K8SIsServiceExportKey = "K8SIsServiceExport"
K8SHTTPRouteNameKey = "K8SHTTPRouteName"
K8SHTTPRouteNamespaceKey = "K8SHTTPRouteNamespace"
K8SIsServiceExport = "true"
K8SIsNotServiceExport = "false"
Copy link
Contributor

@ellistarn ellistarn Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
K8SIsServiceExport = "true"
K8SIsNotServiceExport = "false"
TrueString = "true"
FalseString = "false"
)

type TargetGroup struct {
core.ResourceMeta `json:"-"`
Spec TargetGroupSpec `json:"spec"`
Expand All @@ -25,6 +35,12 @@ type TargetGroupConfig struct {
VpcID string `json:"vpcid"`
EKSClusterName string `json:"eksclustername"`
IsServiceImport bool `json:"serviceimport"`
// the following fields are used for AWS resource tagging
IsServiceExport bool `json:"serviceexport"`
K8SServiceName string `json:"k8sservice"`
K8SServiceNamespace string `json:"k8sservicenamespace"`
K8SHTTPRouteName string `json:"k8shttproutename"`
K8SHTTPRouteNamespace string `json:"k8shttproutenamespace"`
Comment on lines +39 to +43
Copy link
Contributor

@ellistarn ellistarn Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct in thinking that this is a user facing API?

  1. We shouldn't be using k8s in the api field name
  2. The field should use camelcase
  3. Instead of hardcoding name and namespace, we should use something like this
ref: kind: HTTPRoute name: myname namespace: mynamespace
Copy link
Contributor Author

@liwenwu-amazon liwenwu-amazon Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these fields are internal.
}

type TargetGroupStatus struct {
Expand Down