Skip to content

Commit

Permalink
Add rbac role and process finalizers
Browse files Browse the repository at this point in the history
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
  • Loading branch information
cnvergence committed Jun 18, 2023
1 parent 78aa4af commit 9d4cfa1
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 34 deletions.
1 change: 1 addition & 0 deletions charts/gateway-helm/templates/generated/rbac/roles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ rules:
verbs:
- get
- list
- patch
- update
- watch
- apiGroups:
Expand Down
51 changes: 26 additions & 25 deletions internal/provider/kubernetes/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand All @@ -36,6 +35,7 @@ import (
"github.com/envoyproxy/gateway/internal/message"
"github.com/envoyproxy/gateway/internal/provider/utils"
"github.com/envoyproxy/gateway/internal/status"
"github.com/envoyproxy/gateway/internal/utils/slice"
)

const (
Expand Down Expand Up @@ -154,7 +154,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.
// The gatewayclass was marked for deletion and the finalizer removed,
// so clean-up dependents.
if !gwClass.DeletionTimestamp.IsZero() &&
!controllerutil.ContainsFinalizer(&gwClass, gatewayClassFinalizer) {
!slice.ContainsString(gwClass.Finalizers, gatewayClassFinalizer) {
r.log.Info("gatewayclass marked for deletion")
cc.removeMatch(&gwClass)

Expand Down Expand Up @@ -249,7 +249,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.
}

// Process the parametersRef of the accepted GatewayClass.
if acceptedGC.Spec.ParametersRef != nil && acceptedGC.DeletionTimestamp == nil {
if acceptedGC.Spec.ParametersRef != nil {
if err := r.processParamsRef(ctx, acceptedGC, resourceTree); err != nil {
msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err)
if err := r.gatewayClassUpdater(ctx, acceptedGC, false, string(gwapiv1b1.GatewayClassReasonInvalidParameters), msg); err != nil {
Expand All @@ -268,27 +268,13 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.
// Update finalizer on the gateway class based on the resource tree.
if len(resourceTree.Gateways) == 0 {
r.log.Info("No gateways found for accepted gatewayclass")
if refsEnvoyProxy(acceptedGC) {
if err := r.removeFinalizer(ctx, resourceTree.EnvoyProxy); err != nil {
r.log.Error(err, fmt.Sprintf("failed to remove finalizer from envoy proxy %s",
resourceTree.EnvoyProxy.Name))
return reconcile.Result{}, err
}
}
// If needed, remove the finalizer from the accepted GatewayClass.
if err := r.removeFinalizer(ctx, acceptedGC); err != nil {
r.log.Error(err, fmt.Sprintf("failed to remove finalizer from gatewayclass %s",
acceptedGC.Name))
return reconcile.Result{}, err
}
} else {
if refsEnvoyProxy(acceptedGC) {
if err := r.addFinalizer(ctx, resourceTree.EnvoyProxy); err != nil {
r.log.Error(err, fmt.Sprintf("failed adding finalizer to envoy proxy %s",
resourceTree.EnvoyProxy.Name))
return reconcile.Result{}, err
}
}
// finalize the accepted GatewayClass.
if err := r.addFinalizer(ctx, acceptedGC); err != nil {
r.log.Error(err, fmt.Sprintf("failed adding finalizer to gatewayclass %s",
Expand Down Expand Up @@ -884,18 +870,18 @@ func secretGatewayIndexFunc(rawObj client.Object) []string {
func (r *gatewayAPIReconciler) addFinalizer(ctx context.Context, obj client.Object) error {
switch objType := obj.(type) {
case *gwapiv1b1.GatewayClass:
if !controllerutil.ContainsFinalizer(objType, gatewayClassFinalizer) {
if !slice.ContainsString(objType.Finalizers, gatewayClassFinalizer) {
base := client.MergeFrom(objType.DeepCopy())
controllerutil.AddFinalizer(objType, gatewayClassFinalizer)
objType.Finalizers = append(objType.Finalizers, gatewayClassFinalizer)
if err := r.client.Patch(ctx, objType, base); err != nil {
return fmt.Errorf("failed to add finalizer to Gateway Class %s: %w", objType.Name, err)
}
}
return nil
case *egcfgv1a1.EnvoyProxy:
if !controllerutil.ContainsFinalizer(objType, gatewayClassFinalizer) {
if !slice.ContainsString(objType.Finalizers, gatewayClassFinalizer) {
base := client.MergeFrom(objType.DeepCopy())
controllerutil.AddFinalizer(objType, gatewayClassFinalizer)
objType.Finalizers = append(objType.Finalizers, gatewayClassFinalizer)
if err := r.client.Patch(ctx, objType, base); err != nil {
return fmt.Errorf("failed to add finalizer to Envoy Proxy %s: %w", objType.Name, err)
}
Expand All @@ -910,18 +896,18 @@ func (r *gatewayAPIReconciler) addFinalizer(ctx context.Context, obj client.Obje
func (r *gatewayAPIReconciler) removeFinalizer(ctx context.Context, obj client.Object) error {
switch objType := obj.(type) {
case *gwapiv1b1.GatewayClass:
if controllerutil.ContainsFinalizer(objType, gatewayClassFinalizer) {
if slice.ContainsString(objType.Finalizers, gatewayClassFinalizer) {
base := client.MergeFrom(objType.DeepCopy())
controllerutil.RemoveFinalizer(objType, gatewayClassFinalizer)
objType.Finalizers = slice.RemoveString(objType.Finalizers, gatewayClassFinalizer)
if err := r.client.Patch(ctx, objType, base); err != nil {
return fmt.Errorf("failed to add finalizer to Gateway Class %s: %w", objType.Name, err)
}
}
return nil
case *egcfgv1a1.EnvoyProxy:
if controllerutil.ContainsFinalizer(objType, gatewayClassFinalizer) {
if slice.ContainsString(objType.Finalizers, gatewayClassFinalizer) {
base := client.MergeFrom(objType.DeepCopy())
controllerutil.RemoveFinalizer(objType, gatewayClassFinalizer)
objType.Finalizers = slice.RemoveString(objType.Finalizers, gatewayClassFinalizer)
if err := r.client.Patch(ctx, objType, base); err != nil {
return fmt.Errorf("failed to add finalizer to Envoy Proxy %s: %w", objType.Name, err)
}
Expand Down Expand Up @@ -1331,12 +1317,27 @@ func (r *gatewayAPIReconciler) processParamsRef(ctx context.Context, gc *gwapiv1
for i := range epList.Items {
ep := epList.Items[i]
r.log.Info("processing envoyproxy", "namespace", ep.Namespace, "name", ep.Name)

if !gc.DeletionTimestamp.IsZero() {
if err := r.removeFinalizer(ctx, &ep); err != nil {
r.log.Error(err, fmt.Sprintf("failed to remove finalizer from envoy proxy %s",
ep.Name))
return err
}
return nil
}

if classRefsEnvoyProxy(gc, &ep) {
found = true
if err := (&ep).Validate(); err != nil {
validationErr = fmt.Errorf("invalid envoyproxy: %v", err)
continue
}
if err := r.addFinalizer(ctx, &ep); err != nil {
r.log.Error(err, fmt.Sprintf("failed adding finalizer to envoy proxy %s",
ep.Name))
return err
}
valid = true
resourceTree.EnvoyProxy = &ep
break
Expand Down
5 changes: 3 additions & 2 deletions internal/provider/kubernetes/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,9 @@ func TestProcessParamsRef(t *testing.T) {
},
ep: &egcfgv1a1.EnvoyProxy{
ObjectMeta: metav1.ObjectMeta{
Namespace: config.DefaultNamespace,
Name: "test",
Namespace: config.DefaultNamespace,
Name: "test",
Finalizers: []string{gatewayClassFinalizer},
},
},
expected: true,
Expand Down
6 changes: 0 additions & 6 deletions internal/provider/kubernetes/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,6 @@ func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *P
return false
}, defaultWait, defaultTick)

// Ensure the envoyproxy has been finalized.
require.Eventually(t, func() bool {
err := cli.Get(ctx, types.NamespacedName{Name: ep.Name}, ep)
return err == nil && slices.Contains(ep.Finalizers, gatewayClassFinalizer)
}, defaultWait, defaultTick)

// Ensure the resource map contains the EnvoyProxy.
res, ok := resources.GatewayAPIResources.Load(gc.Name)
assert.Equal(t, ok, true)
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/kubernetes/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package kubernetes
// +kubebuilder:rbac:groups="gateway.networking.k8s.io",resources=gatewayclasses/status;gateways/status;httproutes/status;grpcroutes/status;tlsroutes/status;tcproutes/status;udproutes/status,verbs=update

// RBAC for Envoy Gateway custom resources.
// +kubebuilder:rbac:groups="config.gateway.envoyproxy.io",resources=envoyproxies,verbs=get;list;watch;update
// +kubebuilder:rbac:groups="config.gateway.envoyproxy.io",resources=envoyproxies,verbs=get;list;watch;update;patch
// +kubebuilder:rbac:groups="gateway.envoyproxy.io",resources=authenticationfilters;ratelimitfilters,verbs=get;list;watch;update

// RBAC for watched resources of Gateway API controllers.
Expand Down
29 changes: 29 additions & 0 deletions internal/utils/slice/slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package slice

// ContainsString checks if a given slice of strings contains the provided string.
func ContainsString(slice []string, s string) bool {
for _, item := range slice {
if item == s {
return true
}
}
return false
}

// RemoveString returns a newly created []string that contains all items from slice that
// are not equal to s.
func RemoveString(slice []string, s string) []string {
var newSlice []string
for _, item := range slice {
if item == s {
continue
}
newSlice = append(newSlice, item)
}
return newSlice
}
63 changes: 63 additions & 0 deletions internal/utils/slice/slice_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package slice

import (
"reflect"
"testing"
)

func TestContainsString(t *testing.T) {
src := []string{"aa", "bb", "cc"}

if !ContainsString(src, "bb") {
t.Errorf("ContainsString didn't find the string as expected")
}

src = make([]string, 0)
if ContainsString(src, "") {
t.Errorf("The result returned is not the expected result")
}
}

func TestRemoveString(t *testing.T) {
tests := []struct {
testName string
input []string
remove string
want []string
}{
{
testName: "Nil input slice",
input: nil,
remove: "",
want: nil,
},
{
testName: "Remove a string from input slice",
input: []string{"a", "ab", "cdef"},
remove: "ab",
want: []string{"a", "cdef"},
},
{
testName: "Slice doesn't contain the string",
input: []string{"a", "ab", "cdef"},
remove: "NotPresentInSlice",
want: []string{"a", "ab", "cdef"},
},
{
testName: "All strings removed, result is nil",
input: []string{"a"},
remove: "a",
want: nil,
},
}
for _, tt := range tests {
if got := RemoveString(tt.input, tt.remove); !reflect.DeepEqual(got, tt.want) {
t.Errorf("%v: RemoveString(%v, %q) = %v WANT %v", tt.testName, tt.input, tt.remove, got, tt.want)
}
}
}

0 comments on commit 9d4cfa1

Please sign in to comment.