diff --git a/apis/projectcontour/v1/httpproxy.go b/apis/projectcontour/v1/httpproxy.go index b0a999fd921..1de37a7248e 100644 --- a/apis/projectcontour/v1/httpproxy.go +++ b/apis/projectcontour/v1/httpproxy.go @@ -1514,3 +1514,6 @@ type SlowStartPolicy struct { // +kubebuilder:validation:Maximum=100 MinimumWeightPercent uint32 `json:"minWeightPercent"` } + +// +kubebuilder:validation:Enum=grpcroutes;tlsroutes;extensionservices;backendtlspolicies +type Feature string diff --git a/apis/projectcontour/v1/httpproxy_helpers.go b/apis/projectcontour/v1/httpproxy_helpers.go deleted file mode 100644 index 2716fa01738..00000000000 --- a/apis/projectcontour/v1/httpproxy_helpers.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1 - -func NamespacesToStrings(ns []Namespace) []string { - res := make([]string, len(ns)) - for i, n := range ns { - res[i] = string(n) - } - return res -} diff --git a/apis/projectcontour/v1alpha1/contourdeployment.go b/apis/projectcontour/v1alpha1/contourdeployment.go index 049a098df42..da534f384fa 100644 --- a/apis/projectcontour/v1alpha1/contourdeployment.go +++ b/apis/projectcontour/v1alpha1/contourdeployment.go @@ -131,6 +131,14 @@ type ContourSettings struct { // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=42 WatchNamespaces []contour_api_v1.Namespace `json:"watchNamespaces,omitempty"` + + // DisabledFeatures defines an array of resources that will be ignored by + // contour reconciler. + // +optional + // +kubebuilder:validation:Type=array + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=42 + DisabledFeatures []contour_api_v1.Feature `json:"disabledFeatures,omitempty"` } // DeploymentSettings contains settings for Deployment resources. diff --git a/changelogs/unreleased/6152-lubronzhan-minor.md b/changelogs/unreleased/6152-lubronzhan-minor.md new file mode 100644 index 00000000000..116189f08cc --- /dev/null +++ b/changelogs/unreleased/6152-lubronzhan-minor.md @@ -0,0 +1,7 @@ +## Add DisabledFeatures to ContourDeployment for gateway provisioner + +A new flag DisabledFeatures is added to ContourDeployment so that user can configure contour which is deployed by the provisioner to skip reconciling CRDs which are specified inside the flag. + +Accepted values are `grpcroutes|tlsroutes|extensionservices|backendtlspolicies`. + + diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 9de3a3bf2bb..6695ac9b884 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -1479,6 +1479,20 @@ spec: type: string type: object type: object + disabledFeatures: + description: |- + DisabledFeatures defines an array of resources that will be ignored by + contour reconciler. + items: + enum: + - grpcroutes + - tlsroutes + - extensionservices + - backendtlspolicies + type: string + maxItems: 42 + minItems: 1 + type: array kubernetesLogLevel: description: |- KubernetesLogLevel Enable Kubernetes client debug logging with log level. If unset, diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index d79153ad41b..f401e80ddeb 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -1698,6 +1698,20 @@ spec: type: string type: object type: object + disabledFeatures: + description: |- + DisabledFeatures defines an array of resources that will be ignored by + contour reconciler. + items: + enum: + - grpcroutes + - tlsroutes + - extensionservices + - backendtlspolicies + type: string + maxItems: 42 + minItems: 1 + type: array kubernetesLogLevel: description: |- KubernetesLogLevel Enable Kubernetes client debug logging with log level. If unset, diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index 9d6a8e51877..79a2d1a5830 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -1490,6 +1490,20 @@ spec: type: string type: object type: object + disabledFeatures: + description: |- + DisabledFeatures defines an array of resources that will be ignored by + contour reconciler. + items: + enum: + - grpcroutes + - tlsroutes + - extensionservices + - backendtlspolicies + type: string + maxItems: 42 + minItems: 1 + type: array kubernetesLogLevel: description: |- KubernetesLogLevel Enable Kubernetes client debug logging with log level. If unset, diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 9bc2b73bdcc..ab80e2eaf68 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -1701,6 +1701,20 @@ spec: type: string type: object type: object + disabledFeatures: + description: |- + DisabledFeatures defines an array of resources that will be ignored by + contour reconciler. + items: + enum: + - grpcroutes + - tlsroutes + - extensionservices + - backendtlspolicies + type: string + maxItems: 42 + minItems: 1 + type: array kubernetesLogLevel: description: |- KubernetesLogLevel Enable Kubernetes client debug logging with log level. If unset, diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index 4b8212d47e4..73c2bf28dd4 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -1698,6 +1698,20 @@ spec: type: string type: object type: object + disabledFeatures: + description: |- + DisabledFeatures defines an array of resources that will be ignored by + contour reconciler. + items: + enum: + - grpcroutes + - tlsroutes + - extensionservices + - backendtlspolicies + type: string + maxItems: 42 + minItems: 1 + type: array kubernetesLogLevel: description: |- KubernetesLogLevel Enable Kubernetes client debug logging with log level. If unset, diff --git a/internal/provisioner/controller/gateway.go b/internal/provisioner/controller/gateway.go index f83c74f396f..c258fab9a44 100644 --- a/internal/provisioner/controller/gateway.go +++ b/internal/provisioner/controller/gateway.go @@ -17,7 +17,6 @@ import ( "context" "fmt" - contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/gatewayapi" "github.com/projectcontour/contour/internal/provisioner/model" @@ -262,7 +261,9 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct contourModel.Spec.KubernetesLogLevel = contourParams.KubernetesLogLevel - contourModel.Spec.WatchNamespaces = contour_api_v1.NamespacesToStrings(contourParams.WatchNamespaces) + contourModel.Spec.WatchNamespaces = contourParams.WatchNamespaces + + contourModel.Spec.DisabledFeatures = contourParams.DisabledFeatures if contourParams.Deployment != nil && contourParams.Deployment.Strategy != nil { diff --git a/internal/provisioner/model/model.go b/internal/provisioner/model/model.go index 35541eb57bc..d6258ceb93a 100644 --- a/internal/provisioner/model/model.go +++ b/internal/provisioner/model/model.go @@ -14,6 +14,7 @@ package model import ( + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" contourv1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/ref" @@ -253,7 +254,27 @@ type ContourSpec struct { // WatchNamespaces is an array of namespaces. Setting it will instruct the contour instance // to only watch these set of namespaces // default is nil, contour will watch resource of all namespaces - WatchNamespaces []string + WatchNamespaces []contourv1.Namespace + + // DisabledFeatures defines an array of resources that will be ignored by + // contour reconciler. + DisabledFeatures []contourv1.Feature +} + +func NamespacesToStrings(ns []contourv1.Namespace) []string { + res := make([]string, len(ns)) + for i, n := range ns { + res[i] = string(n) + } + return res +} + +func FeaturesToStrings(fs []contourv1.Feature) []string { + res := make([]string, len(fs)) + for i := range fs { + res[i] = string(fs[i]) + } + return res } // WorkloadType is the type of Kubernetes workload to use for a component. diff --git a/apis/projectcontour/v1/httpproxy_helpers_test.go b/internal/provisioner/model/model_test.go similarity index 52% rename from apis/projectcontour/v1/httpproxy_helpers_test.go rename to internal/provisioner/model/model_test.go index bacfaa55d4f..61800672bd9 100644 --- a/apis/projectcontour/v1/httpproxy_helpers_test.go +++ b/internal/provisioner/model/model_test.go @@ -11,27 +11,29 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1 +package model import ( "reflect" "testing" + + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" ) func TestNamespacesToStrings(t *testing.T) { testCases := []struct { description string - namespaces []Namespace + namespaces []contourv1.Namespace expectStrings []string }{ { - description: "namespace 1", - namespaces: []Namespace{}, + description: "no namespaces", + namespaces: []contourv1.Namespace{}, expectStrings: []string{}, }, { - description: "namespace 2", - namespaces: []Namespace{"ns1", "ns2"}, + description: "2 namespaces", + namespaces: []contourv1.Namespace{"ns1", "ns2"}, expectStrings: []string{"ns1", "ns2"}, }, } @@ -44,3 +46,30 @@ func TestNamespacesToStrings(t *testing.T) { }) } } + +func TestFeaturesToStrings(t *testing.T) { + testCases := []struct { + description string + features []contourv1.Feature + expectStrings []string + }{ + { + description: "no features", + features: []contourv1.Feature{}, + expectStrings: []string{}, + }, + { + description: "2 features", + features: []contourv1.Feature{"tlsroutes", "grpcroutes"}, + expectStrings: []string{"tlsroutes", "grpcroutes"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + if !reflect.DeepEqual(FeaturesToStrings(tc.features), tc.expectStrings) { + t.Errorf("expect converted strings %v is the same as %v", FeaturesToStrings(tc.features), tc.expectStrings) + } + }) + } +} diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go index bfc56116bcb..ec1700c6eb5 100644 --- a/internal/provisioner/objects/deployment/deployment.go +++ b/internal/provisioner/objects/deployment/deployment.go @@ -103,13 +103,17 @@ func DesiredDeployment(contour *model.Contour, image string) *appsv1.Deployment } if !contour.WatchAllNamespaces() { - ns := contour.Spec.WatchNamespaces - if !slices.Contains(contour.Spec.WatchNamespaces, contour.Namespace) { + ns := model.NamespacesToStrings(contour.Spec.WatchNamespaces) + if !slices.Contains(ns, contour.Namespace) { ns = append(ns, contour.Namespace) } args = append(args, fmt.Sprintf("--watch-namespaces=%s", strings.Join(ns, ","))) } + if contour.Spec.DisabledFeatures != nil && len(contour.Spec.DisabledFeatures) > 0 { + args = append(args, fmt.Sprintf("--disable-feature=%s", strings.Join(model.FeaturesToStrings(contour.Spec.DisabledFeatures), ","))) + } + // Pass the insecure/secure flags to Contour if using non-default ports. for _, port := range contour.Spec.NetworkPublishing.Envoy.Ports { switch { diff --git a/internal/provisioner/objects/deployment/deployment_test.go b/internal/provisioner/objects/deployment/deployment_test.go index 71d0c81be6f..9cf62aa29ed 100644 --- a/internal/provisioner/objects/deployment/deployment_test.go +++ b/internal/provisioner/objects/deployment/deployment_test.go @@ -18,6 +18,7 @@ import ( "strings" "testing" + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/provisioner/model" @@ -216,15 +217,15 @@ func TestDesiredDeployment(t *testing.T) { func TestDesiredDeploymentWhenSettingWatchNamespaces(t *testing.T) { testCases := []struct { description string - namespaces []string + namespaces []contourv1.Namespace }{ { description: "several valid namespaces", - namespaces: []string{"ns1", "ns2"}, + namespaces: []contourv1.Namespace{"ns1", "ns2"}, }, { description: "single valid namespace", - namespaces: []string{"ns1"}, + namespaces: []contourv1.Namespace{"ns1"}, }, } @@ -238,7 +239,7 @@ func TestDesiredDeploymentWhenSettingWatchNamespaces(t *testing.T) { cntr.Spec.WatchNamespaces = tc.namespaces deploy := DesiredDeployment(cntr, "ghcr.io/projectcontour/contour:test") container := checkDeploymentHasContainer(t, deploy, contourContainerName, true) - arg := fmt.Sprintf("--watch-namespaces=%s", strings.Join(append(tc.namespaces, cntr.Namespace), ",")) + arg := fmt.Sprintf("--watch-namespaces=%s", strings.Join(append(model.NamespacesToStrings(tc.namespaces), cntr.Namespace), ",")) checkContainerHasArg(t, container, arg) }) } @@ -270,3 +271,34 @@ func TestNodePlacementDeployment(t *testing.T) { checkDeploymentHasNodeSelector(t, deploy, selectors) checkDeploymentHasTolerations(t, deploy, tolerations) } + +func TestDesiredDeploymentWhenSettingDisabledFeature(t *testing.T) { + testCases := []struct { + description string + disabledFeatures []contourv1.Feature + }{ + { + description: "disable 2 featuers", + disabledFeatures: []contourv1.Feature{"tlsroutes", "grpcroutes"}, + }, + { + description: "disable single feature", + disabledFeatures: []contourv1.Feature{"tlsroutes"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + name := "deploy-test" + cntr := model.Default(fmt.Sprintf("%s-ns", name), name) + icName := "test-ic" + cntr.Spec.IngressClassName = &icName + cntr.Spec.DisabledFeatures = tc.disabledFeatures + // Change the Contour watch namespaces flag + deploy := DesiredDeployment(cntr, "ghcr.io/projectcontour/contour:test") + container := checkDeploymentHasContainer(t, deploy, contourContainerName, true) + arg := fmt.Sprintf("--disable-feature=%s", strings.Join(model.FeaturesToStrings(tc.disabledFeatures), ",")) + checkContainerHasArg(t, container, arg) + }) + } +} diff --git a/internal/provisioner/objects/rbac/clusterrole/cluster_role.go b/internal/provisioner/objects/rbac/clusterrole/cluster_role.go index 6371a9202f2..700ca27c31f 100644 --- a/internal/provisioner/objects/rbac/clusterrole/cluster_role.go +++ b/internal/provisioner/objects/rbac/clusterrole/cluster_role.go @@ -58,8 +58,8 @@ func desiredClusterRole(name string, contour *model.Contour, clusterScopedResour return role } - // add basic rules to role - role.Rules = append(role.Rules, util.NamespacedResourcePolicyRules()...) + // add other rules for namespacedResources, so that we can associated them with ClusterRole later + role.Rules = append(role.Rules, util.NamespacedResourcePolicyRules(contour.Spec.DisabledFeatures)...) return role } diff --git a/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go b/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go index 2e48d3edd98..56fa4a8809f 100644 --- a/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go +++ b/internal/provisioner/objects/rbac/clusterrole/cluster_role_test.go @@ -19,9 +19,14 @@ import ( "testing" "github.com/projectcontour/contour/internal/provisioner/model" + "github.com/projectcontour/contour/internal/provisioner/objects/rbac/util" + "github.com/projectcontour/contour/internal/provisioner/slice" + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" rbacv1 "k8s.io/api/rbac/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/utils/diff" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) func checkClusterRoleName(t *testing.T, cr *rbacv1.ClusterRole, expected string) { @@ -82,7 +87,6 @@ func TestDesiredClusterRole(t *testing.T) { model.GatewayAPIOwningGatewayNameLabel: cntr.Name, } checkClusterRoleLabels(t, cr, ownerLabels) - fmt.Println(cr.Rules) if tc.clusterScopeOnly != clusterRoleRulesContainOnlyClusterScopeRules(cr) { t.Errorf("expect clusterScopeOnly to be %v, but clusterRoleRulesContainOnlyClusterScopeRules shows %v", tc.clusterScopeOnly, clusterRoleRulesContainOnlyClusterScopeRules(cr)) @@ -90,3 +94,131 @@ func TestDesiredClusterRole(t *testing.T) { }) } } + +func TestDesiredClusterRoleFilterResources(t *testing.T) { + filterNamespacedGatewayResources := func(policyRules []rbacv1.PolicyRule) [][]string { + gatewayResources := [][]string{} + for _, rule := range policyRules { + for _, apigroup := range rule.APIGroups { + // gatewayclass is in isolate rule + if apigroup == gatewayv1alpha2.GroupName && rule.Resources[0] != "gatewayclasses" && rule.Resources[0] != "gatewayclasses/status" { + gatewayResources = append(gatewayResources, rule.Resources) + break + } + } + } + return gatewayResources + } + + filterContourResources := func(policyRules []rbacv1.PolicyRule) [][]string { + contourResources := [][]string{} + for _, rule := range policyRules { + for _, apigroup := range rule.APIGroups { + if apigroup == contourv1.GroupName { + contourResources = append(contourResources, rule.Resources) + break + } + } + } + return contourResources + } + + tests := []struct { + description string + disabledFeatures []contourv1.Feature + clusterScopedResourceOnly bool + expectedGateway [][]string + expectedContour [][]string + }{ + { + description: "empty disabled features", + disabledFeatures: nil, + clusterScopedResourceOnly: false, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + { + description: "disable tlsroutes feature", + disabledFeatures: []contourv1.Feature{"tlsroutes"}, + clusterScopedResourceOnly: false, + expectedGateway: [][]string{ + removeFromStringArray(util.GatewayGroupNamespacedResource, "tlsroutes"), + removeFromStringArray(util.GatewayGroupNamespacedResourceStatus, "tlsroutes/status"), + }, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + + { + description: "disable extensionservices feature", + disabledFeatures: []contourv1.Feature{"extensionservices"}, + clusterScopedResourceOnly: false, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{ + removeFromStringArray(util.ContourGroupNamespacedResource, "extensionservices"), + removeFromStringArray(util.ContourGroupNamespacedResourceStatus, "extensionservices/status"), + }, + }, + { + description: "disable non-existent features", + disabledFeatures: []contourv1.Feature{"abc", "efg"}, + clusterScopedResourceOnly: false, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + { + description: "disable both gateway and contour features", + disabledFeatures: []contourv1.Feature{"grpcroutes", "tlsroutes", "extensionservices", "backendtlspolicies"}, + clusterScopedResourceOnly: false, + expectedGateway: [][]string{ + removeFromStringArray(util.GatewayGroupNamespacedResource, "tlsroutes", "grpcroutes", "backendtlspolicies"), + removeFromStringArray(util.GatewayGroupNamespacedResourceStatus, "tlsroutes/status", "grpcroutes/status", "backendtlspolicies/status"), + }, + expectedContour: [][]string{ + removeFromStringArray(util.ContourGroupNamespacedResource, "extensionservices"), + removeFromStringArray(util.ContourGroupNamespacedResourceStatus, "extensionservices/status"), + }, + }, + { + description: "empty disabled features but with clusterScoped only", + disabledFeatures: nil, + clusterScopedResourceOnly: true, + expectedGateway: [][]string{}, + expectedContour: [][]string{}, + }, + } + + cntrName := "test-filteredresources" + cntr := model.Default(fmt.Sprintf("%s-ns", cntrName), cntrName) + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + cntrLocal := cntr + + // set the disableFeatures + cntrLocal.Spec.DisabledFeatures = tt.disabledFeatures + + cr := desiredClusterRole(cntrName, cntrLocal, tt.clusterScopedResourceOnly) + + // fetch gateway resources + gatewayResources := filterNamespacedGatewayResources(cr.Rules) + contourResources := filterContourResources(cr.Rules) + if !apiequality.Semantic.DeepEqual(gatewayResources, tt.expectedGateway) { + t.Errorf("filtered gateway resources didn't match: %v", diff.ObjectReflectDiff(gatewayResources, tt.expectedGateway)) + } + + if !apiequality.Semantic.DeepEqual(contourResources, tt.expectedContour) { + t.Errorf("filtered contour resources didn't match: %v", diff.ObjectReflectDiff(contourResources, tt.expectedContour)) + } + }) + } +} + +func removeFromStringArray(arr []string, s ...string) []string { + res := []string{} + for _, a := range arr { + if !slice.ContainsString(s, a) { + res = append(res, a) + } + } + return res +} diff --git a/internal/provisioner/objects/rbac/rbac.go b/internal/provisioner/objects/rbac/rbac.go index 30b731a7e1d..271698c90ff 100644 --- a/internal/provisioner/objects/rbac/rbac.go +++ b/internal/provisioner/objects/rbac/rbac.go @@ -17,6 +17,7 @@ import ( "context" "fmt" + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/provisioner/model" "github.com/projectcontour/contour/internal/provisioner/objects" "github.com/projectcontour/contour/internal/provisioner/objects/rbac/clusterrole" @@ -81,7 +82,7 @@ func ensureContourRBAC(ctx context.Context, cli client.Client, contour *model.Co } // includes contour's namespace if it's not inside watchNamespaces - ns := contour.Spec.WatchNamespaces + ns := model.NamespacesToStrings(contour.Spec.WatchNamespaces) if !slice.ContainsString(ns, contour.Namespace) { ns = append(ns, contour.Namespace) } @@ -178,12 +179,12 @@ func EnsureRBACDeleted(ctx context.Context, cli client.Client, contour *model.Co return nil } -func validateNamespacesExist(ctx context.Context, cli client.Client, ns []string) error { +func validateNamespacesExist(ctx context.Context, cli client.Client, ns []contourv1.Namespace) error { errs := []error{} for _, n := range ns { namespace := &corev1.Namespace{} // Check if the namespace exists - err := cli.Get(ctx, types.NamespacedName{Name: n}, namespace) + err := cli.Get(ctx, types.NamespacedName{Name: string(n)}, namespace) if err != nil { if apierrors.IsNotFound(err) { errs = append(errs, fmt.Errorf("failed to find namespace %s in watchNamespace. Please make sure it exist", n)) diff --git a/internal/provisioner/objects/rbac/role/role.go b/internal/provisioner/objects/rbac/role/role.go index f4dbd6b0a8d..83ab82085ce 100644 --- a/internal/provisioner/objects/rbac/role/role.go +++ b/internal/provisioner/objects/rbac/role/role.go @@ -108,7 +108,7 @@ func desiredRoleForResourceInNamespace(name, namespace string, contour *model.Co Labels: contour.CommonLabels(), Annotations: contour.CommonAnnotations(), }, - Rules: util.NamespacedResourcePolicyRules(), + Rules: util.NamespacedResourcePolicyRules(contour.Spec.DisabledFeatures), } } diff --git a/internal/provisioner/objects/rbac/role/role_test.go b/internal/provisioner/objects/rbac/role/role_test.go index a42fa7b8712..8db45e5f41d 100644 --- a/internal/provisioner/objects/rbac/role/role_test.go +++ b/internal/provisioner/objects/rbac/role/role_test.go @@ -17,10 +17,15 @@ import ( "fmt" "testing" + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/provisioner/model" + "github.com/projectcontour/contour/internal/provisioner/objects/rbac/util" + "github.com/projectcontour/contour/internal/provisioner/slice" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" rbacv1 "k8s.io/api/rbac/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/utils/diff" ) func checkRoleName(t *testing.T, role *rbacv1.Role, expected string) { @@ -95,3 +100,117 @@ func TestDesiredRoleForContourInNamespace(t *testing.T) { }) } } + +func TestDesiredRoleFilterResources(t *testing.T) { + filterNamespacedGatewayResources := func(policyRules []rbacv1.PolicyRule) [][]string { + gatewayResources := [][]string{} + for _, rule := range policyRules { + for _, apigroup := range rule.APIGroups { + if apigroup == gatewayv1alpha2.GroupName { + gatewayResources = append(gatewayResources, rule.Resources) + break + } + } + } + return gatewayResources + } + + filterContourResources := func(policyRules []rbacv1.PolicyRule) [][]string { + contourResources := [][]string{} + for _, rule := range policyRules { + for _, apigroup := range rule.APIGroups { + if apigroup == contourv1.GroupName { + contourResources = append(contourResources, rule.Resources) + break + } + } + } + return contourResources + } + + tests := []struct { + description string + disabledFeatures []contourv1.Feature + expectedGateway [][]string + expectedContour [][]string + }{ + { + description: "empty disabled features", + disabledFeatures: nil, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + { + description: "disable tlsroutes feature", + disabledFeatures: []contourv1.Feature{"tlsroutes"}, + expectedGateway: [][]string{ + removeFromStringArray(util.GatewayGroupNamespacedResource, "tlsroutes"), + removeFromStringArray(util.GatewayGroupNamespacedResourceStatus, "tlsroutes/status"), + }, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + + { + description: "disable extensionservices feature", + disabledFeatures: []contourv1.Feature{"extensionservices"}, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{ + removeFromStringArray(util.ContourGroupNamespacedResource, "extensionservices"), + removeFromStringArray(util.ContourGroupNamespacedResourceStatus, "extensionservices/status"), + }, + }, + { + description: "disable non-existent features", + disabledFeatures: []contourv1.Feature{"abc", "efg"}, + expectedGateway: [][]string{util.GatewayGroupNamespacedResource, util.GatewayGroupNamespacedResourceStatus}, + expectedContour: [][]string{util.ContourGroupNamespacedResource, util.ContourGroupNamespacedResourceStatus}, + }, + { + description: "disable both gateway and contour features", + disabledFeatures: []contourv1.Feature{"grpcroutes", "tlsroutes", "backendtlspolicies", "extensionservices"}, + expectedGateway: [][]string{ + removeFromStringArray(util.GatewayGroupNamespacedResource, "tlsroutes", "grpcroutes", "backendtlspolicies"), + removeFromStringArray(util.GatewayGroupNamespacedResourceStatus, "tlsroutes/status", "grpcroutes/status", "backendtlspolicies/status"), + }, + expectedContour: [][]string{ + removeFromStringArray(util.ContourGroupNamespacedResource, "extensionservices"), + removeFromStringArray(util.ContourGroupNamespacedResourceStatus, "extensionservices/status"), + }, + }, + } + + cntrName := "test-filteredresources" + cntr := model.Default(fmt.Sprintf("%s-ns", cntrName), cntrName) + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + cntrLocal := cntr + + // set the disableFeatures + cntrLocal.Spec.DisabledFeatures = tt.disabledFeatures + + cr := desiredRoleForResourceInNamespace(cntrName, "test", cntrLocal) + + // fetch gateway resources + gatewayResources := filterNamespacedGatewayResources(cr.Rules) + contourResources := filterContourResources(cr.Rules) + if !apiequality.Semantic.DeepEqual(gatewayResources, tt.expectedGateway) { + t.Errorf("filtered gateway resources didn't match: %v", diff.ObjectReflectDiff(gatewayResources, tt.expectedGateway)) + } + + if !apiequality.Semantic.DeepEqual(contourResources, tt.expectedContour) { + t.Errorf("filtered contour resources didn't match: %v", diff.ObjectReflectDiff(contourResources, tt.expectedContour)) + } + }) + } +} + +func removeFromStringArray(arr []string, s ...string) []string { + res := []string{} + for _, a := range arr { + if !slice.ContainsString(s, a) { + res = append(res, a) + } + } + return res +} diff --git a/internal/provisioner/objects/rbac/util/util.go b/internal/provisioner/objects/rbac/util/util.go index 698cb828144..1fb781f0d6d 100644 --- a/internal/provisioner/objects/rbac/util/util.go +++ b/internal/provisioner/objects/rbac/util/util.go @@ -14,6 +14,11 @@ package util import ( + "strings" + + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" + "github.com/projectcontour/contour/internal/provisioner/model" + "github.com/projectcontour/contour/internal/provisioner/slice" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" networkingv1 "k8s.io/api/networking/v1" @@ -23,6 +28,13 @@ import ( const contourV1GroupName = "projectcontour.io" +var ( + GatewayGroupNamespacedResource = []string{"gateways", "httproutes", "tlsroutes", "grpcroutes", "tcproutes", "referencegrants", "backendtlspolicies"} + GatewayGroupNamespacedResourceStatus = []string{"gateways/status", "httproutes/status", "tlsroutes/status", "grpcroutes/status", "tcproutes/status", "backendtlspolicies/status"} + ContourGroupNamespacedResource = []string{"httpproxies", "tlscertificatedelegations", "extensionservices", "contourconfigurations"} + ContourGroupNamespacedResourceStatus = []string{"httpproxies/status", "extensionservices/status", "contourconfigurations/status"} +) + var ( createGetUpdate = []string{"create", "get", "update"} getListWatch = []string{"get", "list", "watch"} @@ -39,8 +51,9 @@ func PolicyRuleFor(apiGroup string, verbs []string, resources ...string) rbacv1. } // NamespacedResourcePolicyRules returns a set of policy rules for resources that are -// namespaced-scoped -func NamespacedResourcePolicyRules() []rbacv1.PolicyRule { +// namespaced-scoped. If resourcesToSkip is not empty, skip creating RBAC for those +// CRDs. +func NamespacedResourcePolicyRules(resourcesToSkip []contourv1.Feature) []rbacv1.PolicyRule { return []rbacv1.PolicyRule{ // Core Contour-watched resources. PolicyRuleFor(corev1.GroupName, getListWatch, "secrets", "endpoints", "services", "configmaps"), @@ -50,16 +63,16 @@ func NamespacedResourcePolicyRules() []rbacv1.PolicyRule { // Gateway API resources. // Note, ReferenceGrant does not currently have a .status field so it's omitted from the status rule. - PolicyRuleFor(gatewayv1alpha2.GroupName, getListWatch, "gateways", "httproutes", "tlsroutes", "grpcroutes", "tcproutes", "referencegrants", "backendtlspolicies"), - PolicyRuleFor(gatewayv1alpha2.GroupName, update, "gateways/status", "httproutes/status", "tlsroutes/status", "grpcroutes/status", "tcproutes/status", "backendtlspolicies/status"), + PolicyRuleFor(gatewayv1alpha2.GroupName, getListWatch, filterResources(resourcesToSkip, GatewayGroupNamespacedResource...)...), + PolicyRuleFor(gatewayv1alpha2.GroupName, update, filterResources(resourcesToSkip, GatewayGroupNamespacedResourceStatus...)...), // Ingress resources. PolicyRuleFor(networkingv1.GroupName, getListWatch, "ingresses"), PolicyRuleFor(networkingv1.GroupName, createGetUpdate, "ingresses/status"), // Contour CRDs. - PolicyRuleFor(contourV1GroupName, getListWatch, "httpproxies", "tlscertificatedelegations", "extensionservices", "contourconfigurations"), - PolicyRuleFor(contourV1GroupName, createGetUpdate, "httpproxies/status", "extensionservices/status", "contourconfigurations/status"), + PolicyRuleFor(contourV1GroupName, getListWatch, filterResources(resourcesToSkip, ContourGroupNamespacedResource...)...), + PolicyRuleFor(contourV1GroupName, createGetUpdate, filterResources(resourcesToSkip, ContourGroupNamespacedResourceStatus...)...), } } @@ -75,3 +88,23 @@ func ClusterScopedResourcePolicyRules() []rbacv1.PolicyRule { PolicyRuleFor(corev1.GroupName, getListWatch, "namespaces"), } } + +func filterResources(resourcesToSkip []contourv1.Feature, resources ...string) []string { + if len(resourcesToSkip) == 0 { + return resources + } + filteredResources := []string{} + rts := model.FeaturesToStrings(resourcesToSkip) + for _, resource := range resources { + resourceCopy := resource + // handle status resources by splitting and using the first part + if strings.Contains(resourceCopy, "/") { + parts := strings.Split(resourceCopy, "/") + resourceCopy = parts[0] + } + if !slice.ContainsString(rts, resourceCopy) { + filteredResources = append(filteredResources, resource) + } + } + return filteredResources +} diff --git a/internal/provisioner/objects/rbac/util/util_test.go b/internal/provisioner/objects/rbac/util/util_test.go new file mode 100644 index 00000000000..c6218761e07 --- /dev/null +++ b/internal/provisioner/objects/rbac/util/util_test.go @@ -0,0 +1,71 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "reflect" + "testing" + + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" +) + +func TestFilterResources(t *testing.T) { + testCases := []struct { + description string + disabledFeatures []contourv1.Feature + resourceList []string + expectedList []string + }{ + { + description: "empty disabled features", + resourceList: []string{"httpproxies", "tlscertificatedelegations", "extensionservices", "contourconfigurations"}, + disabledFeatures: nil, + expectedList: []string{"httpproxies", "tlscertificatedelegations", "extensionservices", "contourconfigurations"}, + }, + { + description: "disable extensionservices", + resourceList: []string{"httpproxies", "tlscertificatedelegations", "extensionservices", "contourconfigurations"}, + disabledFeatures: []contourv1.Feature{"extensionservices"}, + expectedList: []string{"httpproxies", "tlscertificatedelegations", "contourconfigurations"}, + }, + { + description: "disable extensionservices, filter status", + resourceList: []string{"httpproxies/status", "extensionservices/status", "contourconfigurations/status"}, + disabledFeatures: []contourv1.Feature{"extensionservices"}, + expectedList: []string{"httpproxies/status", "contourconfigurations/status"}, + }, + { + description: "disable tlsroutes", + resourceList: []string{"gateways", "httproutes", "tlsroutes", "grpcroutes", "tcproutes", "referencegrants"}, + disabledFeatures: []contourv1.Feature{"tlsroutes"}, + expectedList: []string{"gateways", "httproutes", "grpcroutes", "tcproutes", "referencegrants"}, + }, + { + description: "disable non-existence abc", + resourceList: []string{"gateways", "httproutes", "tlsroutes", "grpcroutes", "tcproutes", "referencegrants"}, + disabledFeatures: []contourv1.Feature{"abc"}, + expectedList: []string{"gateways", "httproutes", "tlsroutes", "grpcroutes", "tcproutes", "referencegrants"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + f := filterResources(tc.disabledFeatures, tc.resourceList...) + if !reflect.DeepEqual(tc.expectedList, f) { + t.Errorf("expect filtered list to be %v, but is %v", + tc.expectedList, f) + } + }) + } +} diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index dde8bb108e2..522e89d16a8 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -1176,6 +1176,14 @@

ExtensionServiceReferenc +

Feature +(string alias)

+

+(Appears on: +ContourSettings) +

+

+

GenericKeyDescriptor

@@ -6284,6 +6292,22 @@

ContourSettings to only watch this subset of namespaces.

+ + +disabledFeatures +
+ + +[]Feature + + + + +(Optional) +

DisabledFeatures defines an array of resources that will be ignored by +contour reconciler.

+ +

CustomTag diff --git a/test/e2e/gatewayapi_predicates.go b/test/e2e/gatewayapi_predicates.go index aa1946ad860..67556328ce0 100644 --- a/test/e2e/gatewayapi_predicates.go +++ b/test/e2e/gatewayapi_predicates.go @@ -145,6 +145,31 @@ func TCPRouteAccepted(route *gatewayapi_v1alpha2.TCPRoute) bool { return false } +// TLSRouteIgnoredByContour returns true if the route has an empty .status.parents.conditions list +func TLSRouteIgnoredByContour(route *gatewayapi_v1alpha2.TLSRoute) bool { + if route == nil { + return false + } + + return len(route.Status.Parents) == 0 +} + +// TLSRouteAccepted returns true if the route has a .status.conditions +// entry of "Accepted: true". +func TLSRouteAccepted(route *gatewayapi_v1alpha2.TLSRoute) bool { + if route == nil { + return false + } + + for _, gw := range route.Status.Parents { + if conditionExists(gw.Conditions, string(gatewayapi_v1alpha2.RouteConditionAccepted), metav1.ConditionTrue) { + return true + } + } + + return false +} + // BackendTLSPolicyAccepted returns true if the backend TLS policy has a .status.conditions // entry of "Accepted: true". func BackendTLSPolicyAccepted(btp *gatewayapi_v1alpha2.BackendTLSPolicy) bool { diff --git a/test/e2e/provisioner/provisioner_test.go b/test/e2e/provisioner/provisioner_test.go index f1c24740885..9390eb16bab 100644 --- a/test/e2e/provisioner/provisioner_test.go +++ b/test/e2e/provisioner/provisioner_test.go @@ -34,6 +34,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -540,7 +541,7 @@ var _ = Describe("Gateway provisioner", func() { params := &contour_api_v1alpha1.ContourDeployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, - Name: "contour-params-with-watch-namespaces", + Name: objectTestName, }, Spec: contour_api_v1alpha1.ContourDeploymentSpec{ RuntimeSettings: contourDeploymentRuntimeSettings(), @@ -669,6 +670,7 @@ var _ = Describe("Gateway provisioner", func() { // Root proxy in non-watched namespace should fail By(fmt.Sprintf("Expect namespace %s not to be watched by contour", t.namespace)) hr, ok := f.CreateHTTPRouteAndWaitFor(route, e2e.HTTPRouteIgnoredByContour) + require.True(f.T(), ok, fmt.Sprintf("httproute's is %v", hr)) By(fmt.Sprintf("Expect httproute under namespace %s is not accepted for a period of time", t.namespace)) require.Never(f.T(), func() bool { @@ -677,12 +679,132 @@ var _ = Describe("Gateway provisioner", func() { return false } return e2e.HTTPRouteAccepted(hr) - }, 10*time.Second, time.Second, hr) - require.True(f.T(), ok, fmt.Sprintf("httproute's is %v", hr)) + }, 20*time.Second, time.Second, hr) } } }) }, "testns-1", "testns-2", "testns-3") + f.NamespacedTest("gateway-with-envoy-with-disabled-features", func(namespace string) { + objectTestName := "contour-params-with-disabled-features" + BeforeEach(func() { + By("create gatewayclass that reference contourDeployment with disabled-features value") + gatewayClass := &gatewayapi_v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: objectTestName, + }, + Spec: gatewayapi_v1beta1.GatewayClassSpec{ + ControllerName: gatewayapi_v1beta1.GatewayController("projectcontour.io/gateway-controller"), + ParametersRef: &gatewayapi_v1beta1.ParametersReference{ + Group: "projectcontour.io", + Kind: "ContourDeployment", + Namespace: ref.To(gatewayapi_v1beta1.Namespace(namespace)), + Name: objectTestName, + }, + }, + } + _, ok := f.CreateGatewayClassAndWaitFor(gatewayClass, e2e.GatewayClassNotAccepted) + require.True(f.T(), ok) + + // Now create the ContourDeployment to match the parametersRef. + params := &contour_api_v1alpha1.ContourDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: objectTestName, + }, + Spec: contour_api_v1alpha1.ContourDeploymentSpec{ + RuntimeSettings: contourDeploymentRuntimeSettings(), + Contour: &contour_api_v1alpha1.ContourSettings{ + DisabledFeatures: []contour_api_v1.Feature{"tlsroutes"}, + }, + }, + } + require.NoError(f.T(), f.Client.Create(context.Background(), params)) + + // Now the GatewayClass should be accepted. + require.Eventually(f.T(), func() bool { + gc := &gatewayapi_v1beta1.GatewayClass{} + if err := f.Client.Get(context.Background(), k8s.NamespacedNameOf(gatewayClass), gc); err != nil { + return false + } + + return e2e.GatewayClassAccepted(gc) + }, time.Minute, time.Second) + }) + AfterEach(func() { + require.NoError(f.T(), f.DeleteGatewayClass(&gatewayapi_v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: objectTestName, + }, + }, false)) + }) + Specify("A gateway can be provisioned that ignore CRDs in disabledFeatures", func() { + By("Deploy gateway that referencing above gatewayclass") + gateway := &gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tlsroute", + Namespace: namespace, + }, + Spec: gatewayapi_v1beta1.GatewaySpec{ + GatewayClassName: gatewayapi_v1beta1.ObjectName(objectTestName), + Listeners: []gatewayapi_v1beta1.Listener{ + { + Name: "https", + Protocol: gatewayapi_v1.TLSProtocolType, + Port: gatewayapi_v1beta1.PortNumber(443), + TLS: &gatewayapi_v1beta1.GatewayTLSConfig{ + Mode: ptr.To(gatewayapi_v1.TLSModePassthrough), + }, + AllowedRoutes: &gatewayapi_v1beta1.AllowedRoutes{ + Namespaces: &gatewayapi_v1beta1.RouteNamespaces{ + From: ref.To(gatewayapi_v1.NamespacesFromSame), + }, + }, + }, + }, + }, + } + + gateway, ok := f.CreateGatewayAndWaitFor(gateway, func(gw *gatewayapi_v1beta1.Gateway) bool { + return e2e.GatewayProgrammed(gw) && e2e.GatewayHasAddress(gw) + }) + require.True(f.T(), ok, fmt.Sprintf("gateway is %v", gateway)) + + By("Skip reconciling the TLSRoute if disabledFeatures includes it") + f.Fixtures.EchoSecure.Deploy(namespace, "echo-secure", nil) + route := &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "tlsroute-1", + }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + Hostnames: []gatewayapi_v1alpha2.Hostname{"provisioner.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + { + Namespace: ref.To(gatewayapi_v1alpha2.Namespace(gateway.Namespace)), + Name: gatewayapi_v1alpha2.ObjectName(gateway.Name), + }, + }, + }, + Rules: []gatewayapi_v1alpha2.TLSRouteRule{ + { + BackendRefs: gatewayapi.TLSRouteBackendRef("echo-secure", 443, ref.To(int32(1))), + }, + }, + }, + } + tr, ok := f.CreateTLSRouteAndWaitFor(route, e2e.TLSRouteIgnoredByContour) + require.True(f.T(), ok, fmt.Sprintf("tlsroute's is %v", tr)) + By("Expect tlsroute not to be accepted") + require.Never(f.T(), func() bool { + tr = &gatewayapi_v1alpha2.TLSRoute{} + if err := f.Client.Get(context.Background(), k8s.NamespacedNameOf(tr), tr); err != nil { + return false + } + return e2e.TLSRouteAccepted(tr) + }, 20*time.Second, time.Second, tr) + }) + }) }) func contourDeploymentRuntimeSettings() *contour_api_v1alpha1.ContourConfigurationSpec {