Skip to content

Commit

Permalink
Change getAttachedTargetGroupPolicy to a generic method `getAttachedP…
Browse files Browse the repository at this point in the history
…olicy()` (#404)

* Change getAttachedTargetGroupPolicy to a generic method `getAttachedPolicy()`

* Address PR comments

---------

Co-authored-by: Zijun Wang <[email protected]>
  • Loading branch information
zijun726911 and Zijun Wang authored Sep 20, 2023
1 parent 0999983 commit 5204483
Show file tree
Hide file tree
Showing 6 changed files with 304 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/aws/aws-application-networking-k8s/pkg/k8s"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
)

const (
Expand Down Expand Up @@ -136,3 +137,11 @@ func (p *TargetGroupPolicy) GetTargetRef() *v1alpha2.PolicyTargetReference {
func (p *TargetGroupPolicy) GetNamespacedName() types.NamespacedName {
return k8s.NamespacedName(p)
}

func (pl *TargetGroupPolicyList) GetItems() []core.Policy {
items := make([]core.Policy, len(pl.Items))
for i, item := range pl.Items {
items[i] = &item
}
return items
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/aws/aws-application-networking-k8s/pkg/k8s"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
)

const (
Expand Down Expand Up @@ -69,3 +70,11 @@ func (p *VpcAssociationPolicy) GetTargetRef() *v1alpha2.PolicyTargetReference {
func (p *VpcAssociationPolicy) GetNamespacedName() types.NamespacedName {
return k8s.NamespacedName(p)
}

func (pl *VpcAssociationPolicyList) GetItems() []core.Policy {
items := make([]core.Policy, len(pl.Items))
for i, item := range pl.Items {
items[i] = &item
}
return items
}
45 changes: 7 additions & 38 deletions pkg/gateway/model_build_targetgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (

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

"k8s.io/apimachinery/pkg/api/meta"

"github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
lattice_aws "github.com/aws/aws-application-networking-k8s/pkg/aws"
"github.com/aws/aws-application-networking-k8s/pkg/config"
Expand Down Expand Up @@ -218,7 +216,7 @@ func (t *targetGroupModelBuildTask) buildTargetGroupForServiceExportCreation(ctx
return nil, err
}

tgp, err := getAttachedTargetGroupPolicy(ctx, t.client, t.serviceExport.Name, t.serviceExport.Namespace)
tgp, err := GetAttachedPolicy(ctx, t.client, k8s.NamespacedName(t.serviceExport), &v1alpha1.TargetGroupPolicy{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -422,7 +420,12 @@ func (t *latticeServiceModelBuildTask) buildTargetGroupSpec(

tgName := latticestore.TargetGroupName(string(backendRef.Name()), namespace)

tgp, err := getAttachedTargetGroupPolicy(ctx, client, string(backendRef.Name()), namespace)
refObjNamespacedName := types.NamespacedName{
Namespace: namespace,
Name: string(backendRef.Name()),
}
tgp, err := GetAttachedPolicy(ctx, t.client, refObjNamespacedName, &v1alpha1.TargetGroupPolicy{})

if err != nil {
return latticemodel.TargetGroupSpec{}, err
}
Expand Down Expand Up @@ -476,40 +479,6 @@ func (t *latticeServiceModelBuildTask) buildTargetGroupName(_ context.Context, b
}
}

func getAttachedTargetGroupPolicy(ctx context.Context, k8sClient client.Client, svcName, svcNamespace string) (*v1alpha1.TargetGroupPolicy, error) {
policyList := &v1alpha1.TargetGroupPolicyList{}
err := k8sClient.List(ctx, policyList, &client.ListOptions{
Namespace: svcNamespace,
})
if err != nil {
if meta.IsNoMatchError(err) {
// CRD does not exist
return nil, nil
}
return nil, err
}
for _, policy := range policyList.Items {
targetRef := policy.Spec.TargetRef
if targetRef == nil {
continue
}

groupKindMatch := targetRef.Group == "" && targetRef.Kind == "Service"
nameMatch := string(targetRef.Name) == svcName

namespace := policy.Namespace
if targetRef.Namespace != nil {
namespace = string(*targetRef.Namespace)
}
namespaceMatch := namespace == svcNamespace

if groupKindMatch && nameMatch && namespaceMatch {
return &policy, nil
}
}
return nil, nil
}

func parseHealthCheckConfig(tgp *v1alpha1.TargetGroupPolicy) *vpclattice.HealthCheckConfig {
hc := tgp.Spec.HealthCheck
if hc == nil {
Expand Down
63 changes: 63 additions & 0 deletions pkg/gateway/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package gateway

import (
"context"
"fmt"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gateway_api "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
)

func GetAttachedPolicy[T core.Policy](ctx context.Context, k8sClient client.Client, refObjNamespacedName types.NamespacedName, policy T) (T, error) {
null := *new(T)
policyList, expectedTargetRefObjGroup, expectedTargetRefObjKind, err := policyTypeToPolicyListAndTargetRefGroupKind(policy)
if err != nil {
return null, err
}

err = k8sClient.List(ctx, policyList.(client.ObjectList), &client.ListOptions{
Namespace: refObjNamespacedName.Namespace,
})
if err != nil {
if meta.IsNoMatchError(err) {
// CRD does not exist
return null, nil
}
return null, err
}
for _, p := range policyList.GetItems() {
targetRef := p.GetTargetRef()
if targetRef == nil {
continue
}
groupKindMatch := targetRef.Group == expectedTargetRefObjGroup && targetRef.Kind == expectedTargetRefObjKind
nameMatch := string(targetRef.Name) == refObjNamespacedName.Name

retrievedNamespace := p.GetNamespacedName().Namespace
if targetRef.Namespace != nil {
retrievedNamespace = string(*targetRef.Namespace)
}
namespaceMatch := retrievedNamespace == refObjNamespacedName.Namespace
if groupKindMatch && nameMatch && namespaceMatch {
return p.(T), nil
}
}
return null, nil
}

func policyTypeToPolicyListAndTargetRefGroupKind(policyType core.Policy) (core.PolicyList, gateway_api.Group, gateway_api.Kind, error) {
switch policyType.(type) {
case *v1alpha1.VpcAssociationPolicy:
return &v1alpha1.VpcAssociationPolicyList{}, gateway_api.GroupName, "Gateway", nil
case *v1alpha1.TargetGroupPolicy:
return &v1alpha1.TargetGroupPolicyList{}, corev1.GroupName, "Service", nil
default:
return nil, "", "", fmt.Errorf("unsupported policy type %T", policyType)
}
}
212 changes: 212 additions & 0 deletions pkg/gateway/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
package gateway

import (
"context"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
gateway_api_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

mock_client "github.com/aws/aws-application-networking-k8s/mocks/controller-runtime/client"
"github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
)

func Test_getAttachedPolicy(t *testing.T) {
type args struct {
refObjNamespacedName types.NamespacedName
policy core.Policy
}
type testCase struct {
name string
args args
expectedK8sClientReturnedPolicy core.Policy
want core.Policy
expectPolicyNotFound bool
}
ns := "test-ns"
typedNs := gateway_api_v1alpha2.Namespace(ns)
prtocol := "HTTP"
protocolVersion := "HTTP2"
trueBool := true
targetGroupPolicyHappyPath := &v1alpha1.TargetGroupPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test-target-group-policy",
Namespace: ns,
},
Spec: v1alpha1.TargetGroupPolicySpec{
TargetRef: &gateway_api_v1alpha2.PolicyTargetReference{
Group: corev1.GroupName,
Name: "svc-1",
Kind: "Service",
Namespace: &typedNs,
},
Protocol: &prtocol,
ProtocolVersion: &protocolVersion,
},
}

policyTargetRefSectionNil := targetGroupPolicyHappyPath.DeepCopyObject().(*v1alpha1.TargetGroupPolicy)
policyTargetRefSectionNil.Spec.TargetRef = nil

policyTargetRefKindWrong := targetGroupPolicyHappyPath.DeepCopyObject().(*v1alpha1.TargetGroupPolicy)
policyTargetRefKindWrong.Spec.TargetRef.Kind = "ServiceImport"

notRelatedTargetGroupPolicy := targetGroupPolicyHappyPath.DeepCopyObject().(*v1alpha1.TargetGroupPolicy)
notRelatedTargetGroupPolicy.Spec.TargetRef.Name = "another-svc"

vpcAssociationPolicyHappyPath := &v1alpha1.VpcAssociationPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test-vpc-association-policy",
Namespace: ns,
},
Spec: v1alpha1.VpcAssociationPolicySpec{
TargetRef: &gateway_api_v1alpha2.PolicyTargetReference{
Group: gateway_api_v1alpha2.GroupName,
Name: "gw-1",
Kind: "Gateway",
},
SecurityGroupIds: []v1alpha1.SecurityGroupId{"sg-1", "sg-2"},
AssociateWithVpc: &trueBool,
},
}

notRelatedVpcAssociationPolicy := vpcAssociationPolicyHappyPath.DeepCopyObject().(*v1alpha1.VpcAssociationPolicy)
notRelatedVpcAssociationPolicy.Spec.TargetRef.Name = "another-gw"

var tests = []testCase{
{
name: "Get k8sService attached TargetGroupPolicy, happy path",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "svc-1",
},
policy: &v1alpha1.TargetGroupPolicy{},
},
expectedK8sClientReturnedPolicy: targetGroupPolicyHappyPath,
want: targetGroupPolicyHappyPath,
expectPolicyNotFound: false,
},
{
name: "Get k8sService attached TargetGroupPolicy, policy not found due to input refObj name mismatch",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "another-svc",
},
policy: &v1alpha1.TargetGroupPolicy{},
},
want: nil,
expectedK8sClientReturnedPolicy: targetGroupPolicyHappyPath,
expectPolicyNotFound: true,
},
{
name: "Get k8sService attached TargetGroupPolicy, policy not found due to cluster don't have matched policy",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "svc-1",
},
policy: &v1alpha1.TargetGroupPolicy{},
},
want: nil,
expectedK8sClientReturnedPolicy: nil,
expectPolicyNotFound: true,
},
{
name: "Get k8sService attached TargetGroupPolicy, policy not found due to policy targetRef section is nil",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "svc-1",
},
policy: &v1alpha1.TargetGroupPolicy{},
},
expectedK8sClientReturnedPolicy: policyTargetRefSectionNil,
want: nil,
expectPolicyNotFound: true,
},
{
name: "Get k8sService attached TargetGroupPolicy, policy not found due to policy targetRef Kind mismatch",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "svc-1",
},
policy: &v1alpha1.TargetGroupPolicy{},
},
expectedK8sClientReturnedPolicy: policyTargetRefKindWrong,
want: nil,
expectPolicyNotFound: true,
},
{
name: "Get k8sGateway attached VpcAssociationPolicy, happy path",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "gw-1",
},
policy: &v1alpha1.VpcAssociationPolicy{},
},
expectedK8sClientReturnedPolicy: vpcAssociationPolicyHappyPath,
want: vpcAssociationPolicyHappyPath,
expectPolicyNotFound: false,
},
{
name: "Get k8sGateway attached VpcAssociationPolicy, Not found",
args: args{
refObjNamespacedName: types.NamespacedName{
Namespace: ns,
Name: "gw-1",
},
policy: &v1alpha1.VpcAssociationPolicy{},
},
expectedK8sClientReturnedPolicy: nil,
want: nil,
expectPolicyNotFound: true,
},
}
c := gomock.NewController(t)
defer c.Finish()
ctx := context.TODO()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k8sClient := mock_client.NewMockClient(c)

if _, ok := tt.args.policy.(*v1alpha1.TargetGroupPolicy); ok {
k8sClient.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
func(ctx context.Context, list *v1alpha1.TargetGroupPolicyList, arg3 ...interface{}) error {
list.Items = append(list.Items, *notRelatedTargetGroupPolicy)
if tt.expectedK8sClientReturnedPolicy != nil {
policy := tt.expectedK8sClientReturnedPolicy.(*v1alpha1.TargetGroupPolicy)
list.Items = append(list.Items, *policy)
}
return nil
})
} else if _, ok := tt.args.policy.(*v1alpha1.VpcAssociationPolicy); ok {
k8sClient.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
func(ctx context.Context, list *v1alpha1.VpcAssociationPolicyList, arg3 ...interface{}) error {
list.Items = append(list.Items, *notRelatedVpcAssociationPolicy)
if tt.expectedK8sClientReturnedPolicy != nil {
policy := tt.expectedK8sClientReturnedPolicy.(*v1alpha1.VpcAssociationPolicy)
list.Items = append(list.Items, *policy)
}
return nil
})
}

got, err := GetAttachedPolicy(ctx, k8sClient, tt.args.refObjNamespacedName, tt.args.policy)
if tt.expectPolicyNotFound {
assert.Nil(t, err)
assert.Nil(t, got)
return
}
assert.Equalf(t, tt.want, got, "GetAttachedPolicy(%v, %v, %v, %v)", ctx, k8sClient, tt.args.refObjNamespacedName, tt.args.policy)
})
}
}
4 changes: 4 additions & 0 deletions pkg/model/core/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ type Policy interface {
GetNamespacedName() types.NamespacedName
GetTargetRef() *v1alpha2.PolicyTargetReference
}

type PolicyList interface {
GetItems() []Policy
}

0 comments on commit 5204483

Please sign in to comment.