Resolve comments

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

Author: lubronzhan

apis/projectcontour/v1/httpproxy.go

@@ -42,6 +42,27 @@ type HTTPProxySpec struct {
 	IngressClassName string `json:"ingressClassName,omitempty"`
 }
 
+// Namespace refers to a Kubernetes namespace. It must be a RFC 1123 label.
+//
+// This validation is based off of the corresponding Kubernetes validation:
+// https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L187
+//
+// This is used for Namespace name validation here:
+// https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/api/validation/generic.go#L63
+//
+// Valid values include:
+//
+// * "example"
+//
+// Invalid values include:
+//
+// * "example.com" - "." is an invalid character
+//
+// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`
+// +kubebuilder:validation:MinLength=1
+// +kubebuilder:validation:MaxLength=63
+type Namespace string
+
 // Include describes a set of policies that can be applied to an HTTPProxy in a namespace.
 type Include struct {
 	// Name of the HTTPProxy

apis/projectcontour/v1alpha1/contourdeployment_helpers.go renamed to apis/projectcontour/v1/httpproxy_helpers.go

@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package v1alpha1
+package v1
 
 func NamespacesToStrings(ns []Namespace) []string {
 	res := make([]string, len(ns))

apis/projectcontour/v1alpha1/contourdeployment_helpers_test.go renamed to apis/projectcontour/v1/httpproxy_helpers_test.go

@@ -11,14 +11,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package v1alpha1
+package v1
 
 import (
 	"reflect"
 	"testing"
 )
 
-func TestDesiredRoleBindingInNamespace(t *testing.T) {
+func TestNamespacesToStrings(t *testing.T) {
 	testCases := []struct {
 		description   string
 		namespaces    []Namespace

apis/projectcontour/v1alpha1/contourdeployment.go

@@ -14,6 +14,7 @@
 package v1alpha1
 
 import (
+	contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -75,27 +76,6 @@ type ContourDeploymentSpec struct {
 	ResourceLabels map[string]string `json:"resourceLabels,omitempty"`
 }
 
-// Namespace refers to a Kubernetes namespace. It must be a RFC 1123 label.
-//
-// This validation is based off of the corresponding Kubernetes validation:
-// https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L187
-//
-// This is used for Namespace name validation here:
-// https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/api/validation/generic.go#L63
-//
-// Valid values include:
-//
-// * "example"
-//
-// Invalid values include:
-//
-// * "example.com" - "." is an invalid character
-//
-// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`
-// +kubebuilder:validation:MinLength=1
-// +kubebuilder:validation:MaxLength=63
-type Namespace string
-
 // ContourSettings contains settings for the Contour part of the installation,
 // i.e. the xDS server/control plane and associated resources.
 type ContourSettings struct {
@@ -150,7 +130,7 @@ type ContourSettings struct {
 	// +kubebuilder:validation:Type=array
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=42
-	WatchNamespaces []Namespace `json:"watchNamespaces,omitempty"`
+	WatchNamespaces []contour_api_v1.Namespace `json:"watchNamespaces,omitempty"`
 }
 
 // DeploymentSettings contains settings for Deployment resources.

apis/projectcontour/v1alpha1/zz_generated.deepcopy.go

@@ -114,8 +114,6 @@ github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0X
 github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
-github.com/go-asn1-ber/asn1-ber v1.5.4 h1:vXT6d/FNDiELJnLb6hGNa309LMsrCoYFvpwHDF0+Y1A=
-github.com/go-asn1-ber/asn1-ber v1.5.4/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
 github.com/go-asn1-ber/asn1-ber v1.5.5 h1:MNHlNMBDgEKD4TcKr36vQN68BA00aDfjIt3/bD50WnA=
 github.com/go-asn1-ber/asn1-ber v1.5.5/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
@@ -381,8 +379,6 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8=
 github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0=
-github.com/tsaarni/certyaml v0.9.2 h1:LoRTuajwjJ1CHAJiMv5cpOtwQ05207Oqe6cT9D7WDaQ=
-github.com/tsaarni/certyaml v0.9.2/go.mod h1:s+ErAC1wZ32r1ihSULvR7HXedKKN5HZasdb8Cj8gT9E=
 github.com/tsaarni/certyaml v0.9.3 h1:m8HHbuUzWVUOmv8IQU9HgVZZ8r5ICExKm++54DJKCs0=
 github.com/tsaarni/certyaml v0.9.3/go.mod h1:hhuU1qYr5re488geArUP4gZWqMUMqGlj4HA2qUyGYLk=
 github.com/tsaarni/x500dn v1.0.0 h1:LvaWTkqRpse4VHBhB5uwf3wytokK4vF9IOyNAEyiA+U=

go.sum

@@ -17,6 +17,7 @@ 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"
@@ -261,7 +262,7 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 			contourModel.Spec.KubernetesLogLevel = contourParams.KubernetesLogLevel
 
-			contourModel.Spec.WatchNamespaces = contour_api_v1alpha1.NamespacesToStrings(contourParams.WatchNamespaces)
+			contourModel.Spec.WatchNamespaces = contour_api_v1.NamespacesToStrings(contourParams.WatchNamespaces)
 
 			if contourParams.Deployment != nil &&
 				contourParams.Deployment.Strategy != nil {

internal/provisioner/controller/gateway.go

@@ -135,16 +135,6 @@ func checkDeploymentHasStrategy(t *testing.T, ds *appsv1.Deployment, expected ap
 	t.Errorf("deployment has unexpected strategy %q", expected)
 }
 
-func ensureContainerDoesntHaveArg(t *testing.T, container *corev1.Container, arg string) {
-	t.Helper()
-
-	for _, a := range container.Args {
-		if a == arg {
-			t.Errorf("container has arg %q", arg)
-		}
-	}
-}
-
 func TestDesiredDeployment(t *testing.T) {
 	name := "deploy-test"
 	cntr := model.Default(fmt.Sprintf("%s-ns", name), name)
@@ -225,19 +215,16 @@ func TestDesiredDeployment(t *testing.T) {
 
 func TestDesiredDeploymentWhenSettingWatchNamespaces(t *testing.T) {
 	testCases := []struct {
-		description    string
-		namespaces     []string
-		expectArgExist bool
+		description string
+		namespaces  []string
 	}{
 		{
-			description:    "several valid namespaces",
-			namespaces:     []string{"ns1", "ns2"},
-			expectArgExist: true,
+			description: "several valid namespaces",
+			namespaces:  []string{"ns1", "ns2"},
 		},
 		{
-			description:    "single valid namespace",
-			namespaces:     []string{"ns1"},
-			expectArgExist: true,
+			description: "single valid namespace",
+			namespaces:  []string{"ns1"},
 		},
 	}
 
@@ -252,11 +239,7 @@ func TestDesiredDeploymentWhenSettingWatchNamespaces(t *testing.T) {
 			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), ","))
-			if tc.expectArgExist {
-				checkContainerHasArg(t, container, arg)
-			} else {
-				ensureContainerDoesntHaveArg(t, container, arg)
-			}
+			checkContainerHasArg(t, container, arg)
 		})
 	}
 }

internal/provisioner/objects/deployment/deployment_test.go

@@ -472,21 +472,6 @@ func (f *Framework) DeleteGatewayClass(gwc *gatewayapi_v1beta1.GatewayClass, wai
 	return nil
 }
 
-// DeleteHTTPRoute deletes the provided HTTPROUTE in the
-// Kubernetes API or fails the test if it encounters an error.
-func (f *Framework) DeleteHTTPRoute(r *gatewayapi_v1beta1.HTTPRoute, waitForDeletion bool) error {
-	require.NoError(f.t, f.Client.Delete(context.TODO(), r))
-
-	if waitForDeletion {
-		require.Eventually(f.t, func() bool {
-			err := f.Client.Get(context.TODO(), client.ObjectKeyFromObject(r), r)
-			return api_errors.IsNotFound(err)
-		}, time.Minute*3, time.Millisecond*50)
-	}
-
-	return nil
-}
-
 // GetEchoResponseBody decodes an HTTP response body that is
 // expected to have come from ingress-conformance-echo into an
 // EchoResponseBody, or fails the test if it encounters an error.

test/e2e/framework.go

@@ -25,6 +25,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	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/k8s"
@@ -519,7 +520,7 @@ var _ = Describe("Gateway provisioner", func() {
 				Spec: contour_api_v1alpha1.ContourDeploymentSpec{
 					RuntimeSettings: contourDeploymentRuntimeSettings(),
 					Contour: &contour_api_v1alpha1.ContourSettings{
-						WatchNamespaces: []contour_api_v1alpha1.Namespace{"testns-1", "testns-2"},
+						WatchNamespaces: []contour_api_v1.Namespace{"testns-1", "testns-2"},
 					},
 				},
 			}
@@ -643,17 +644,15 @@ 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.HTTPRouteIngnoredByContour)
-					By(fmt.Sprintf("Expect httproute under namespace %s is not accepted", t.namespace))
-					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 {
-						r := &gatewayapi_v1beta1.HTTPRoute{}
-						if err := f.Client.Get(context.Background(), k8s.NamespacedNameOf(hr), r); err != nil {
+						hr = &gatewayapi_v1beta1.HTTPRoute{}
+						if err := f.Client.Get(context.Background(), k8s.NamespacedNameOf(hr), hr); err != nil {
 							return false
 						}
-						return e2e.HTTPRouteAccepted(r)
-					}, 10*time.Second, time.Second)
+						return e2e.HTTPRouteAccepted(hr)
+					}, 10*time.Second, time.Second, hr)
 					require.True(f.T(), ok, fmt.Sprintf("httproute's is %v", hr))
 				}
 			}