From 91e595dcf858b657befc554d48c8468df5f32825 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Wed, 15 Nov 2023 11:15:32 -0700 Subject: [PATCH] internal/provisioner: add Gateway label to generated resources Adds the gateway.networking.k8s.io/gateway-name label to generated resources. Signed-off-by: Steve Kriss --- changelogs/unreleased/5969-skriss-small.md | 1 + .../provisioner/controller/gateway_test.go | 55 +++++++++++++++++++ internal/provisioner/labels/labels.go | 19 ++++--- internal/provisioner/labels/labels_test.go | 35 +++++++----- internal/provisioner/model/model.go | 22 +++----- .../contourconfig/contourconfig_test.go | 4 +- .../objects/dataplane/dataplane.go | 4 +- .../objects/deployment/deployment.go | 2 +- internal/provisioner/objects/object.go | 2 +- .../objects/rbac/clusterrole/cluster_role.go | 2 +- .../rbac/clusterrole/cluster_role_test.go | 3 +- .../cluster_role_binding.go | 2 +- .../cluster_role_binding_test.go | 3 +- .../provisioner/objects/rbac/role/role.go | 2 +- .../objects/rbac/role/role_test.go | 3 +- .../objects/rbac/rolebinding/role_binding.go | 2 +- .../rbac/rolebinding/role_binding_test.go | 3 +- .../rbac/serviceaccount/service_account.go | 2 +- .../provisioner/objects/service/service.go | 4 +- 19 files changed, 118 insertions(+), 52 deletions(-) create mode 100644 changelogs/unreleased/5969-skriss-small.md diff --git a/changelogs/unreleased/5969-skriss-small.md b/changelogs/unreleased/5969-skriss-small.md new file mode 100644 index 00000000000..064a7f3f4df --- /dev/null +++ b/changelogs/unreleased/5969-skriss-small.md @@ -0,0 +1 @@ +Gateway API: add the `gateway.networking.k8s.io/gateway-name` label to generated resources. \ No newline at end of file diff --git a/internal/provisioner/controller/gateway_test.go b/internal/provisioner/controller/gateway_test.go index 1d5cad7aa20..d786789e867 100644 --- a/internal/provisioner/controller/gateway_test.go +++ b/internal/provisioner/controller/gateway_test.go @@ -19,6 +19,7 @@ import ( contourv1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/provisioner" + "github.com/projectcontour/contour/internal/provisioner/model" "github.com/projectcontour/contour/internal/ref" "github.com/go-logr/logr" @@ -1343,6 +1344,60 @@ func TestGatewayReconcile(t *testing.T) { } }, }, + "Gateway owner labels are set on all resources": { + gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller), + gateway: makeGateway(), + assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1beta1.Gateway, reconcileErr error) { + require.NoError(t, reconcileErr) + + for _, obj := range []client.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contour-gateway-1"}, + }, + &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "envoy-gateway-1"}, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contour-gateway-1"}, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "envoy-gateway-1"}, + }, + &contourv1alpha1.ContourConfiguration{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contourconfig-gateway-1"}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contourcert-gateway-1"}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "envoycert-gateway-1"}, + }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contour-gateway-1"}, + }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "envoy-gateway-1"}, + }, + &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "contour-gateway-1-gateway-1"}, + }, + &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "contour-gateway-1-gateway-1"}, + }, + &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contour-gateway-1"}, + }, + &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Namespace: "gateway-1", Name: "contour-rolebinding-gateway-1"}, + }, + } { + require.NoError(t, r.client.Get(context.Background(), keyFor(obj), obj)) + + assert.Equal(t, gw.Name, obj.GetLabels()[model.ContourOwningGatewayNameLabel]) + assert.Equal(t, gw.Name, obj.GetLabels()[model.GatewayAPIOwningGatewayNameLabel]) + } + }, + }, } for name, tc := range tests { diff --git a/internal/provisioner/labels/labels.go b/internal/provisioner/labels/labels.go index 08741c24ec2..a0d863948e2 100644 --- a/internal/provisioner/labels/labels.go +++ b/internal/provisioner/labels/labels.go @@ -17,16 +17,19 @@ type LabeledObject interface { GetLabels() map[string]string } -// Exist returns true if obj contains labels m. -func Exist(obj LabeledObject, m map[string]string) bool { - labels := obj.GetLabels() - if labels == nil { +// AnyExist returns true if obj contains at least one of the provided labels. +func AnyExist(obj LabeledObject, labels map[string]string) bool { + objLabels := obj.GetLabels() + + if len(objLabels) == 0 { return false } - for key, val := range m { - if found, ok := labels[key]; !ok || found != val { - return false + + for k, v := range labels { + if val, ok := objLabels[k]; ok && val == v { + return true } } - return true + + return false } diff --git a/internal/provisioner/labels/labels_test.go b/internal/provisioner/labels/labels_test.go index c8f794b1d19..74fccc3d74b 100644 --- a/internal/provisioner/labels/labels_test.go +++ b/internal/provisioner/labels/labels_test.go @@ -17,15 +17,28 @@ import ( "testing" "github.com/projectcontour/contour/internal/provisioner/model" + "github.com/stretchr/testify/assert" ) -func TestExist(t *testing.T) { +func TestAnyExist(t *testing.T) { testCases := []struct { description string current map[string]string exist map[string]string expected bool }{ + { + description: "nil labels", + current: nil, + exist: map[string]string{"name": "foo"}, + expected: false, + }, + { + description: "empty labels", + current: map[string]string{}, + exist: map[string]string{"name": "foo"}, + expected: false, + }, { description: "one matched label", current: map[string]string{"name": "foo"}, @@ -36,7 +49,7 @@ func TestExist(t *testing.T) { description: "one of two matched labels", current: map[string]string{"name": "foo"}, exist: map[string]string{"name": "foo", "ns": "foo-ns"}, - expected: false, + expected: true, }, { description: "two matched labels", @@ -56,20 +69,14 @@ func TestExist(t *testing.T) { exist: map[string]string{"foo": "bar"}, expected: false, }, - { - description: "two unmatched labels", - current: map[string]string{"name": "bar"}, - exist: map[string]string{"name": "bar", "ns": "foo-ns"}, - expected: false, - }, } - contour := model.Contour{} for _, tc := range testCases { - contour.Labels = tc.current - result := Exist(&contour, tc.exist) - if result != tc.expected { - t.Fatalf("%q: returned %t, expected %t.", tc.description, result, tc.expected) - } + t.Run(tc.description, func(t *testing.T) { + contour := model.Contour{} + contour.Labels = tc.current + + assert.Equal(t, tc.expected, AnyExist(&contour, tc.exist)) + }) } } diff --git a/internal/provisioner/model/model.go b/internal/provisioner/model/model.go index a42f6a6d585..026802104b7 100644 --- a/internal/provisioner/model/model.go +++ b/internal/provisioner/model/model.go @@ -24,9 +24,13 @@ import ( ) const ( - // OwningGatewayNameLabel is the owner reference label used for a Contour - // created by the gateway provisioner. The value should be the name of the Gateway. - OwningGatewayNameLabel = "projectcontour.io/owning-gateway-name" + // ContourOwningGatewayNameLabel is the Contour-defined owner reference label applied + // to generated resources. The value should be the name of the Gateway. + ContourOwningGatewayNameLabel = "projectcontour.io/owning-gateway-name" + + // GatewayAPIOwningGatewayNameLabel is the Gateway API-defined owner reference label applied + // to generated resources. The value should be the name of the Gateway. + GatewayAPIOwningGatewayNameLabel = "gateway.networking.k8s.io/gateway-name" ) // Default returns a default instance of a Contour @@ -600,18 +604,10 @@ const ( ContourAvailableConditionType = "Available" ) -// OwningSelector returns a label selector using "projectcontour.io/owning-gateway-name". -func OwningSelector(contour *Contour) *metav1.LabelSelector { - return &metav1.LabelSelector{ - MatchLabels: map[string]string{ - OwningGatewayNameLabel: contour.Name, - }, - } -} - // OwnerLabels returns owner labels for the provided contour. func OwnerLabels(contour *Contour) map[string]string { return map[string]string{ - OwningGatewayNameLabel: contour.Name, + ContourOwningGatewayNameLabel: contour.Name, + GatewayAPIOwningGatewayNameLabel: contour.Name, } } diff --git a/internal/provisioner/objects/contourconfig/contourconfig_test.go b/internal/provisioner/objects/contourconfig/contourconfig_test.go index 6f5487076da..d02f41940f4 100644 --- a/internal/provisioner/objects/contourconfig/contourconfig_test.go +++ b/internal/provisioner/objects/contourconfig/contourconfig_test.go @@ -241,7 +241,7 @@ func TestEnsureContourConfigDeleted(t *testing.T) { Namespace: "contour-namespace", Name: "contourconfig-contour-1", Labels: map[string]string{ - model.OwningGatewayNameLabel: "contour-1", + model.ContourOwningGatewayNameLabel: "contour-1", }, }, }, @@ -292,7 +292,7 @@ func TestEnsureContourConfigDeleted(t *testing.T) { Namespace: "contour-namespace", Name: "contourconfig-contour-1", Labels: map[string]string{ - model.OwningGatewayNameLabel: "some-other-contour", + model.ContourOwningGatewayNameLabel: "some-other-contour", }, }, }, diff --git a/internal/provisioner/objects/dataplane/dataplane.go b/internal/provisioner/objects/dataplane/dataplane.go index 66ec309c1b1..765919aba98 100644 --- a/internal/provisioner/objects/dataplane/dataplane.go +++ b/internal/provisioner/objects/dataplane/dataplane.go @@ -488,7 +488,7 @@ func desiredDeployment(contour *model.Contour, contourImage, envoyImage string) // updateDaemonSetIfNeeded updates a DaemonSet if current does not match desired, // using contour to verify the existence of owner labels. func updateDaemonSetIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *appsv1.DaemonSet) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { ds, updated := equality.DaemonsetConfigChanged(current, desired) if updated { if err := cli.Update(ctx, ds); err != nil { @@ -503,7 +503,7 @@ func updateDaemonSetIfNeeded(ctx context.Context, cli client.Client, contour *mo // updateDeploymentIfNeeded updates a Deployment if current does not match desired, // using contour to verify the existence of owner labels. func updateDeploymentIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *appsv1.Deployment) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { ds, updated := equality.DeploymentConfigChanged(current, desired) if updated { if err := cli.Update(ctx, ds); err != nil { diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go index 50f27f89970..8a14230aa04 100644 --- a/internal/provisioner/objects/deployment/deployment.go +++ b/internal/provisioner/objects/deployment/deployment.go @@ -269,7 +269,7 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment // updateDeploymentIfNeeded updates a Deployment if current does not match desired, // using contour to verify the existence of owner labels. func updateDeploymentIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *appsv1.Deployment) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { deploy, updated := equality.DeploymentConfigChanged(current, desired) if updated { if err := cli.Update(ctx, deploy); err != nil { diff --git a/internal/provisioner/objects/object.go b/internal/provisioner/objects/object.go index bce42f4e4c6..4a963983517 100644 --- a/internal/provisioner/objects/object.go +++ b/internal/provisioner/objects/object.go @@ -98,7 +98,7 @@ func EnsureObjectDeleted[T client.Object](ctx context.Context, cli client.Client return err } - if !labels.Exist(obj, model.OwnerLabels(contour)) { + if !labels.AnyExist(obj, model.OwnerLabels(contour)) { return nil } diff --git a/internal/provisioner/objects/rbac/clusterrole/cluster_role.go b/internal/provisioner/objects/rbac/clusterrole/cluster_role.go index 7238a912644..b610bc7e687 100644 --- a/internal/provisioner/objects/rbac/clusterrole/cluster_role.go +++ b/internal/provisioner/objects/rbac/clusterrole/cluster_role.go @@ -99,7 +99,7 @@ func desiredClusterRole(name string, contour *model.Contour) *rbacv1.ClusterRole // updateClusterRoleIfNeeded updates a ClusterRole resource if current does not match desired, // using contour to verify the existence of owner labels. func updateClusterRoleIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *rbacv1.ClusterRole) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { cr, updated := equality.ClusterRoleConfigChanged(current, desired) if updated { if err := cli.Update(ctx, cr); err != nil { diff --git a/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go b/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go index 89ebbd833a4..2f6a961d32f 100644 --- a/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go +++ b/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go @@ -49,7 +49,8 @@ func TestDesiredClusterRole(t *testing.T) { cr := desiredClusterRole(name, cntr) checkClusterRoleName(t, cr, name) ownerLabels := map[string]string{ - model.OwningGatewayNameLabel: cntr.Name, + model.ContourOwningGatewayNameLabel: cntr.Name, + model.GatewayAPIOwningGatewayNameLabel: cntr.Name, } checkClusterRoleLabels(t, cr, ownerLabels) } diff --git a/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding.go b/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding.go index cd7c257d4fa..a9991a5b3cc 100644 --- a/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding.go +++ b/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding.go @@ -75,7 +75,7 @@ func desiredClusterRoleBinding(name, roleRef, svcAcctRef string, contour *model. // updateClusterRoleBindingIfNeeded updates a ClusterRoleBinding resource if current // does not match desired, using contour to verify the existence of owner labels. func updateClusterRoleBindingIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *rbacv1.ClusterRoleBinding) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { crb, updated := equality.ClusterRoleBindingConfigChanged(current, desired) if updated { if err := cli.Update(ctx, crb); err != nil { diff --git a/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding_test.go b/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding_test.go index f8f81f1846b..822c59bf069 100644 --- a/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding_test.go +++ b/internal/provisioner/objects/rbac/clusterrolebinding/cluster_role_binding_test.go @@ -71,7 +71,8 @@ func TestDesiredClusterRoleBinding(t *testing.T) { crb := desiredClusterRoleBinding(name, testRoleRef, testSvcAcct, cntr) checkClusterRoleBindingName(t, crb, name) ownerLabels := map[string]string{ - model.OwningGatewayNameLabel: cntr.Name, + model.ContourOwningGatewayNameLabel: cntr.Name, + model.GatewayAPIOwningGatewayNameLabel: cntr.Name, } checkClusterRoleBindingLabels(t, crb, ownerLabels) checkClusterRoleBindingSvcAcct(t, crb, testSvcAcct, cntr.Namespace) diff --git a/internal/provisioner/objects/rbac/role/role.go b/internal/provisioner/objects/rbac/role/role.go index f6ca2e6ae49..d326348036c 100644 --- a/internal/provisioner/objects/rbac/role/role.go +++ b/internal/provisioner/objects/rbac/role/role.go @@ -75,7 +75,7 @@ func desiredControllerRole(name string, contour *model.Contour) *rbacv1.Role { // updateRoleIfNeeded updates a Role resource if current does not match desired, // using contour to verify the existence of owner labels. func updateRoleIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *rbacv1.Role) (*rbacv1.Role, error) { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { role, updated := equality.RoleConfigChanged(current, desired) if updated { if err := cli.Update(ctx, role); err != nil { diff --git a/internal/provisioner/objects/rbac/role/role_test.go b/internal/provisioner/objects/rbac/role/role_test.go index 15613223088..4412708bbcc 100644 --- a/internal/provisioner/objects/rbac/role/role_test.go +++ b/internal/provisioner/objects/rbac/role/role_test.go @@ -49,7 +49,8 @@ func TestDesiredControllerRole(t *testing.T) { role := desiredControllerRole(name, cntr) checkRoleName(t, role, name) ownerLabels := map[string]string{ - model.OwningGatewayNameLabel: cntr.Name, + model.ContourOwningGatewayNameLabel: cntr.Name, + model.GatewayAPIOwningGatewayNameLabel: cntr.Name, } checkRoleLabels(t, role, ownerLabels) } diff --git a/internal/provisioner/objects/rbac/rolebinding/role_binding.go b/internal/provisioner/objects/rbac/rolebinding/role_binding.go index bf975930cff..19315dac051 100644 --- a/internal/provisioner/objects/rbac/rolebinding/role_binding.go +++ b/internal/provisioner/objects/rbac/rolebinding/role_binding.go @@ -76,7 +76,7 @@ func desiredRoleBinding(name, svcAcctRef, roleRef string, contour *model.Contour // updateRoleBindingIfNeeded updates a RoleBinding resource if current does // not match desired. func updateRoleBindingIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *rbacv1.RoleBinding) error { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { rb, updated := equality.RoleBindingConfigChanged(current, desired) if updated { if err := cli.Update(ctx, rb); err != nil { diff --git a/internal/provisioner/objects/rbac/rolebinding/role_binding_test.go b/internal/provisioner/objects/rbac/rolebinding/role_binding_test.go index f7c8135ce76..557c94eb36b 100644 --- a/internal/provisioner/objects/rbac/rolebinding/role_binding_test.go +++ b/internal/provisioner/objects/rbac/rolebinding/role_binding_test.go @@ -72,7 +72,8 @@ func TestDesiredRoleBinding(t *testing.T) { rb := desiredRoleBinding(rbName, svcAcct, roleRef, cntr) checkRoleBindingName(t, rb, rbName) ownerLabels := map[string]string{ - model.OwningGatewayNameLabel: cntr.Name, + model.ContourOwningGatewayNameLabel: cntr.Name, + model.GatewayAPIOwningGatewayNameLabel: cntr.Name, } checkRoleBindingLabels(t, rb, ownerLabels) checkRoleBindingSvcAcct(t, rb, svcAcct, cntr.Namespace) diff --git a/internal/provisioner/objects/rbac/serviceaccount/service_account.go b/internal/provisioner/objects/rbac/serviceaccount/service_account.go index de46371060e..26ad2218053 100644 --- a/internal/provisioner/objects/rbac/serviceaccount/service_account.go +++ b/internal/provisioner/objects/rbac/serviceaccount/service_account.go @@ -60,7 +60,7 @@ func desiredServiceAccount(name string, contour *model.Contour) *corev1.ServiceA // updateSvcAcctIfNeeded updates a ServiceAccount resource if current does not match desired, // using contour to verify the existence of owner labels. func updateSvcAcctIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *corev1.ServiceAccount) (*corev1.ServiceAccount, error) { - if labels.Exist(current, model.OwnerLabels(contour)) { + if labels.AnyExist(current, model.OwnerLabels(contour)) { sa, updated := utilequality.ServiceAccountConfigChanged(current, desired) if updated { if err := cli.Update(ctx, sa); err != nil { diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go index 58382875297..f74a8273291 100644 --- a/internal/provisioner/objects/service/service.go +++ b/internal/provisioner/objects/service/service.go @@ -305,7 +305,7 @@ func DesiredEnvoyService(contour *model.Contour) *corev1.Service { // updateContourServiceIfNeeded updates a Contour Service if current does not match desired. func updateContourServiceIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *corev1.Service) error { - if !labels.Exist(current, model.OwnerLabels(contour)) { + if !labels.AnyExist(current, model.OwnerLabels(contour)) { return nil } _, updated := equality.ClusterIPServiceChanged(current, desired) @@ -323,7 +323,7 @@ func updateContourServiceIfNeeded(ctx context.Context, cli client.Client, contou // updateEnvoyServiceIfNeeded updates an Envoy Service if current does not match desired, // using contour to verify the existence of owner labels. func updateEnvoyServiceIfNeeded(ctx context.Context, cli client.Client, contour *model.Contour, current, desired *corev1.Service) error { - if !labels.Exist(current, model.OwnerLabels(contour)) { + if !labels.AnyExist(current, model.OwnerLabels(contour)) { return nil }