Resolve comments

Signed-off-by: lubronzhan <lubronzhan@gmail.com>

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 {

internal/provisioner/model/model.go

@@ -254,13 +254,21 @@ 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 Gateway API CRDs 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 {

apis/projectcontour/v1/httpproxy_helpers_test.go → 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)
+			}
+		})
+	}
+}

internal/provisioner/objects/deployment/deployment.go

@@ -103,8 +103,8 @@ 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, ",")))

internal/provisioner/objects/deployment/deployment_test.go

@@ -18,7 +18,7 @@ import (
 	"strings"
 	"testing"
 
-	contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
+	contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	"github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	"github.com/projectcontour/contour/internal/provisioner/model"
 
@@ -217,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"},
 		},
 	}
 
@@ -239,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)
 		})
 	}
@@ -275,15 +275,15 @@ func TestNodePlacementDeployment(t *testing.T) {
 func TestDesiredDeploymentWhenSettingDisabledFeature(t *testing.T) {
 	testCases := []struct {
 		description      string
-		disabledFeatures []contour_v1.Feature
+		disabledFeatures []contourv1.Feature
 	}{
 		{
-			description:      "several disabled features",
-			disabledFeatures: []contour_v1.Feature{"ns1", "ns2"},
+			description:      "disable 2 featuers",
+			disabledFeatures: []contourv1.Feature{"tlsroutes", "grpcroutes"},
 		},
 		{
-			description:      "single disabled features",
-			disabledFeatures: []contour_v1.Feature{"ns1"},
+			description:      "disable single feature",
+			disabledFeatures: []contourv1.Feature{"tlsroutes"},
 		},
 	}
 

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))

test/e2e/provisioner/provisioner_test.go

@@ -679,7 +679,7 @@ var _ = Describe("Gateway provisioner", func() {
 							return false
 						}
 						return e2e.HTTPRouteAccepted(hr)
-					}, 10*time.Second, time.Second, hr)
+					}, 20*time.Second, time.Second, hr)
 				}
 			}
 		})
@@ -768,8 +768,8 @@ var _ = Describe("Gateway provisioner", func() {
 				return e2e.GatewayProgrammed(gw) && e2e.GatewayHasAddress(gw)
 			})
 			require.True(f.T(), ok, fmt.Sprintf("gateway is %v", gateway))
-			By("Skip reconciling TCPRoute if disabledFeatures includes it")
 
+			By("Skip reconciling the TLSRoute if disabledFeatures includes it")
 			f.Fixtures.EchoSecure.Deploy(namespace, "echo-secure", nil)
 			route := &gatewayapi_v1alpha2.TLSRoute{
 				ObjectMeta: metav1.ObjectMeta{
@@ -802,7 +802,7 @@ var _ = Describe("Gateway provisioner", func() {
 					return false
 				}
 				return e2e.TLSRouteAccepted(tr)
-			}, 10*time.Second, time.Second, tr)
+			}, 20*time.Second, time.Second, tr)
 		})
 	})
 })