Skip to content

Commit

Permalink
Resolve comments
Browse files Browse the repository at this point in the history
Signed-off-by: lubronzhan <lubronzhan@gmail.com>
  • Loading branch information
lubronzhan committed Feb 8, 2024
1 parent f0c55d0 commit d1f3b4e
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 18 deletions.
3 changes: 1 addition & 2 deletions internal/provisioner/controller/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -262,7 +261,7 @@ 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

if contourParams.Deployment != nil &&
contourParams.Deployment.Strategy != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/provisioner/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ 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.
Expand Down
24 changes: 24 additions & 0 deletions internal/provisioner/model/model_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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 model

import contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1"

func NamespacesToStrings(ns []contourv1.Namespace) []string {
res := make([]string, len(ns))
for i, n := range ns {
res[i] = string(n)
}
return res
}
48 changes: 48 additions & 0 deletions internal/provisioner/model/model_helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// 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 model

import (
"reflect"
"testing"

contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1"
)

func TestNamespacesToStrings(t *testing.T) {
testCases := []struct {
description string
namespaces []contourv1.Namespace
expectStrings []string
}{
{
description: "namespace 1",
namespaces: []contourv1.Namespace{},
expectStrings: []string{},
},
{
description: "namespace 2",
namespaces: []contourv1.Namespace{"ns1", "ns2"},
expectStrings: []string{"ns1", "ns2"},
},
}

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
if !reflect.DeepEqual(NamespacesToStrings(tc.namespaces), tc.expectStrings) {
t.Errorf("expect converted strings %v is the same as %v", NamespacesToStrings(tc.namespaces), tc.expectStrings)
}
})
}
}
4 changes: 2 additions & 2 deletions internal/provisioner/objects/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ",")))
Expand Down
20 changes: 10 additions & 10 deletions internal/provisioner/objects/deployment/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"},
},
}

Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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"},
},
}

Expand Down
7 changes: 4 additions & 3 deletions internal/provisioner/objects/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit d1f3b4e

Please sign in to comment.