Resolve conmments

Signed-off-by: Lubron Zhan <[email protected]>

apis/projectcontour/v1alpha1/contourdeployment.go

@@ -124,7 +124,7 @@ type ContourSettings struct {
 	PodAnnotations map[string]string `json:"podAnnotations,omitempty"`
 
 	// WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-	// to only watch these set of namespaces
+	// to only watch this subset of namespaces.
 	// +optional
 	WatchNamespaces []string `json:"watchNamespaces,omitempty"`
 }

examples/contour/01-crds.yaml

@@ -1623,7 +1623,7 @@ spec:
                   watchNamespaces:
                     description: |-
                       WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-                      to only watch these set of namespaces
+                      to only watch this subset of namespaces.
                     items:
                       type: string
                     type: array

examples/render/contour-deployment.yaml

@@ -1842,7 +1842,7 @@ spec:
                   watchNamespaces:
                     description: |-
                       WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-                      to only watch these set of namespaces
+                      to only watch this subset of namespaces.
                     items:
                       type: string
                     type: array

examples/render/contour-gateway-provisioner.yaml

@@ -1634,7 +1634,7 @@ spec:
                   watchNamespaces:
                     description: |-
                       WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-                      to only watch these set of namespaces
+                      to only watch this subset of namespaces.
                     items:
                       type: string
                     type: array

examples/render/contour-gateway.yaml

@@ -1845,7 +1845,7 @@ spec:
                   watchNamespaces:
                     description: |-
                       WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-                      to only watch these set of namespaces
+                      to only watch this subset of namespaces.
                     items:
                       type: string
                     type: array

examples/render/contour.yaml

@@ -1842,7 +1842,7 @@ spec:
                   watchNamespaces:
                     description: |-
                       WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-                      to only watch these set of namespaces
+                      to only watch this subset of namespaces.
                     items:
                       type: string
                     type: array

internal/provisioner/objects/rbac/clusterrole/cluster_role.go

@@ -52,14 +52,14 @@ func desiredClusterRole(name string, contour *model.Contour, gatewayClassOnly bo
 			Labels:      contour.CommonLabels(),
 			Annotations: contour.CommonAnnotations(),
 		},
-		Rules: util.ClusterScopePolicyRulesForContour(),
+		Rules: util.ClusterScopedResourcePolicyRules(),
 	}
 	if gatewayClassOnly {
 		return role
 	}
 
 	// add basic rules to role
-	role.Rules = append(role.Rules, util.BasicPolicyRulesForContour()...)
+	role.Rules = append(role.Rules, util.NamespacedResourcePolicyRules()...)
 	return role
 }
 

internal/provisioner/objects/rbac/rbac.go

@@ -60,33 +60,32 @@ func ensureContourRBAC(ctx context.Context, cli client.Client, contour *model.Co
 		if err := clusterrole.EnsureClusterRole(ctx, cli, names.ClusterRole, contour, !clusterRoleForGatewayclassOnly); err != nil {
 			return fmt.Errorf("failed to ensure cluster role %s: %w", names.ClusterRole, err)
 		}
+		if err := clusterrolebinding.EnsureClusterRoleBinding(ctx, cli, names.ClusterRoleBinding, names.ClusterRole, names.ServiceAccount, contour); err != nil {
+			return fmt.Errorf("failed to ensure cluster role binding %s: %w", names.ClusterRoleBinding, err)
+		}
 	} else {
 		// Ensure cluster role & cluster binding for gatewayclass first since it's cluster scope variables
 		if err := clusterrole.EnsureClusterRole(ctx, cli, names.ClusterRole, contour, clusterRoleForGatewayclassOnly); err != nil {
 			return fmt.Errorf("failed to ensure cluster role %s: %w", names.ClusterRole, err)
 		}
+		if err := clusterrolebinding.EnsureClusterRoleBinding(ctx, cli, names.ClusterRoleBinding, names.ClusterRole, names.ServiceAccount, contour); err != nil {
+			return fmt.Errorf("failed to ensure cluster role binding %s: %w", names.ClusterRoleBinding, err)
+		}
 
-		// Ensures role and rolebinding for set of namespaces in contour.spec.watchNamespaces variable
-		// Ensure cluster role & cluster binding for gatewayclass first since it's cluster scope variables
+		// Ensures role and rolebinding for namespaced scope resources in namespaces specified in contour.spec.watchNamespaces variable
 		if err := role.EnsureRolesInNamespaces(ctx, cli, names.Role, contour, contour.Spec.WatchNamespaces); err != nil {
 			return fmt.Errorf("failed to ensure cluster role %s: %w", names.ClusterRole, err)
 		}
-		// Ensures role and rolebinding for set of namespaces in contour.spec.watchNamespaces variable
-		// Ensure cluster role & cluster binding for gatewayclass first since it's cluster scope variables
 		if err := rolebinding.EnsureRoleBindingsInNamespaces(ctx, cli, names.RoleBinding, names.ServiceAccount, names.Role, contour, contour.Spec.WatchNamespaces); err != nil {
 			return fmt.Errorf("failed to ensure cluster role %s: %w", names.ClusterRole, err)
 		}
 	}
 
-	if err := clusterrolebinding.EnsureClusterRoleBinding(ctx, cli, names.ClusterRoleBinding, names.ClusterRole, names.ServiceAccount, contour); err != nil {
-		return fmt.Errorf("failed to ensure cluster role binding %s: %w", names.ClusterRoleBinding, err)
-	}
-
 	// Ensure role & binding.
 	if err := role.EnsureControllerRole(ctx, cli, names.Role, contour); err != nil {
 		return fmt.Errorf("failed to ensure controller role %s/%s: %w", contour.Namespace, names.Role, err)
 	}
-	if err := rolebinding.EnsureRoleBinding(ctx, cli, names.RoleBinding, names.ServiceAccount, names.Role, contour); err != nil {
+	if err := rolebinding.EnsureControllerRoleBinding(ctx, cli, names.RoleBinding, names.ServiceAccount, names.Role, contour); err != nil {
 		return fmt.Errorf("failed to ensure controller role binding %s/%s: %w", contour.Namespace, names.RoleBinding, err)
 	}
 	return nil

internal/provisioner/objects/rbac/role/role.go

@@ -50,7 +50,7 @@ func EnsureControllerRole(ctx context.Context, cli client.Client, name string, c
 func EnsureRolesInNamespaces(ctx context.Context, cli client.Client, name string, contour *model.Contour, namespaces []string) error {
 	errs := []error{}
 	for _, ns := range namespaces {
-		desired := desiredRoleForContourInNamespace(name, ns, contour)
+		desired := desiredRoleForResourceInNamespace(name, ns, contour)
 
 		updater := func(ctx context.Context, cli client.Client, current, desired *rbacv1.Role) error {
 			err := updateRoleIfNeeded(ctx, cli, contour, current, desired)
@@ -95,20 +95,20 @@ func desiredControllerRole(name string, contour *model.Contour) *rbacv1.Role {
 	return role
 }
 
-// desiredRoleForContour constructs an instance of the desired Role resource with the
-// provided ns/name and using contour namespace/name for the corresponding Contour instance
-func desiredRoleForContourInNamespace(name, namespace string, contour *model.Contour) *rbacv1.Role {
+// desiredRoleForResourceInNamespace constructs an instance of the desired Role resource with the
+// provided ns/contour-resource-<name> and using contour namespace/name for the corresponding Contour instance
+func desiredRoleForResourceInNamespace(name, namespace string, contour *model.Contour) *rbacv1.Role {
 	return &rbacv1.Role{
 		TypeMeta: metav1.TypeMeta{
 			Kind: "Role",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
+			Name:        util.ContourResourceName(name),
 			Namespace:   namespace,
 			Labels:      contour.CommonLabels(),
 			Annotations: contour.CommonAnnotations(),
 		},
-		Rules: util.BasicPolicyRulesForContour(),
+		Rules: util.NamespacedResourcePolicyRules(),
 	}
 }
 

internal/provisioner/objects/rbac/role/role_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/projectcontour/contour/internal/provisioner/model"
+	"github.com/projectcontour/contour/internal/provisioner/objects/rbac/util"
 
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -85,8 +86,8 @@ func TestDesiredRoleForContourInNamespace(t *testing.T) {
 		t.Run(tc.description, func(t *testing.T) {
 			name := "role-test"
 			cntr := model.Default(fmt.Sprintf("%s-ns", name), name)
-			role := desiredRoleForContourInNamespace(name, tc.namespace, cntr)
-			checkRoleName(t, role, name)
+			role := desiredRoleForResourceInNamespace(name, tc.namespace, cntr)
+			checkRoleName(t, role, util.ContourResourceName(name))
 			ownerLabels := map[string]string{
 				model.ContourOwningGatewayNameLabel:    cntr.Name,
 				model.GatewayAPIOwningGatewayNameLabel: cntr.Name,

internal/provisioner/objects/rbac/rolebinding/role_binding.go

@@ -21,17 +21,18 @@ import (
 	"github.com/projectcontour/contour/internal/provisioner/labels"
 	"github.com/projectcontour/contour/internal/provisioner/model"
 	"github.com/projectcontour/contour/internal/provisioner/objects"
+	"github.com/projectcontour/contour/internal/provisioner/objects/rbac/util"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// EnsureRoleBinding ensures a RoleBinding resource exists with the provided
+// EnsureControllerRoleBinding ensures a RoleBinding resource exists with the provided
 // ns/name and using contour namespace/name for the owning contour labels.
 // The RoleBinding will use svcAct for the subject and role for the role reference.
-func EnsureRoleBinding(ctx context.Context, cli client.Client, name, svcAct, role string, contour *model.Contour) error {
+func EnsureControllerRoleBinding(ctx context.Context, cli client.Client, name, svcAct, role string, contour *model.Contour) error {
 	desired := desiredRoleBindingInNamespace(name, svcAct, role, contour.Namespace, contour)
 
 	// Enclose contour.
@@ -42,8 +43,8 @@ func EnsureRoleBinding(ctx context.Context, cli client.Client, name, svcAct, rol
 	return objects.EnsureObject(ctx, cli, desired, updater, &rbacv1.RoleBinding{})
 }
 
-// EnsureRoleBindingsInNamespaces ensures a set of RoleBindings resource exist with the provided
-// namespaces/name using contour namespace/name for the owning contour labels.
+// EnsureRoleBindingsInNamespaces ensures a set of RoleBinding resources exist with the provided
+// namespaces/contour-resource-<name> and using contour namespace/name for the owning contour labels.
 // The RoleBindings will use same svcAct for the subject and role for the role reference.
 func EnsureRoleBindingsInNamespaces(ctx context.Context, cli client.Client, name, svcAct, role string, contour *model.Contour, namespaces []string) error {
 	errs := []error{}
@@ -72,7 +73,7 @@ func desiredRoleBindingInNamespace(name, svcAcctRef, roleRef, namespace string,
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:   namespace,
-			Name:        name,
+			Name:        util.ContourResourceName(name),
 			Labels:      contour.CommonLabels(),
 			Annotations: contour.CommonAnnotations(),
 		},

internal/provisioner/objects/rbac/rolebinding/role_binding_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/projectcontour/contour/internal/provisioner/model"
+	"github.com/projectcontour/contour/internal/provisioner/objects/rbac/util"
 
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -96,7 +97,7 @@ func TestDesiredRoleBindingInNamespace(t *testing.T) {
 			svcAcct := "test-svc-acct-ref"
 			roleRef := "test-role-ref"
 			rb := desiredRoleBindingInNamespace(rbName, svcAcct, roleRef, tc.namespace, cntr)
-			checkRoleBindingName(t, rb, rbName)
+			checkRoleBindingName(t, rb, util.ContourResourceName(rbName))
 			checkRoleBindingNamespace(t, rb, tc.namespace)
 			ownerLabels := map[string]string{
 				model.ContourOwningGatewayNameLabel:    cntr.Name,

internal/provisioner/objects/rbac/util/util.go

@@ -14,6 +14,8 @@
 package util
 
 import (
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	networkingv1 "k8s.io/api/networking/v1"
@@ -40,8 +42,9 @@ func PolicyRuleFor(apiGroup string, verbs []string, resources ...string) rbacv1.
 	}
 }
 
-// BasicPolicyRulesForContour returns set of basic rules that contour requires
-func BasicPolicyRulesForContour() []rbacv1.PolicyRule {
+// NamespacedResourcePolicyRules returns a set of policy rules for resources that are
+// namespaced-scoped
+func NamespacedResourcePolicyRules() []rbacv1.PolicyRule {
 	return []rbacv1.PolicyRule{
 		// Core Contour-watched resources.
 		PolicyRuleFor(corev1.GroupName, getListWatch, "secrets", "endpoints", "services", "namespaces"),
@@ -64,11 +67,17 @@ func BasicPolicyRulesForContour() []rbacv1.PolicyRule {
 	}
 }
 
-// ClusterScopePolicyRulesForContour returns set of rules only for cluster scope object
-func ClusterScopePolicyRulesForContour() []rbacv1.PolicyRule {
+// ClusterScopedResourcePolicyRules returns a set of policy rules for
+// cluster-scoped resources.
+func ClusterScopedResourcePolicyRules() []rbacv1.PolicyRule {
 	return []rbacv1.PolicyRule{
 		// GatewayClass only.
 		PolicyRuleFor(gatewayv1alpha2.GroupName, getListWatch, "gatewayclasses"),
 		PolicyRuleFor(gatewayv1alpha2.GroupName, update, "gatewayclasses/status"),
 	}
 }
+
+// ContourResourceName returns names for roles/rolebindings for contour resources
+func ContourResourceName(name string) string {
+	return fmt.Sprintf("contour-resource-%s", name)
+}

site/content/docs/main/config/api-reference.html

@@ -6258,7 +6258,7 @@ <h3 id="projectcontour.io/v1alpha1.ContourSettings">ContourSettings
 <td>
 <em>(Optional)</em>
 <p>WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance
-to only watch these set of namespaces</p>
+to only watch this subset of namespaces.</p>
 </td>
 </tr>
 </tbody>