From ac49aee3e08e8d9fd36aca920e4f5cc78e4f258a Mon Sep 17 00:00:00 2001 From: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com> Date: Wed, 31 Jan 2024 16:06:32 -0500 Subject: [PATCH] provisioner: Set SupportedVersion condition on GatewayClass (#6147) Based on Gateway API CRD bundle-version annotation We will still reconcile the GatewayClass etc. but will set a status condition on the GatewayClass saying this support is best effort. Signed-off-by: Sunjay Bhatia --- .../unreleased/6147-sunjayBhatia-small.md | 1 + examples/gateway-provisioner/01-roles.yaml | 8 + .../render/contour-gateway-provisioner.yaml | 8 + .../provisioner/controller/gatewayclass.go | 139 ++++--- .../controller/gatewayclass_test.go | 387 ++++++++++++++---- internal/provisioner/rbac/rbac.go | 1 + internal/provisioner/scheme.go | 2 + test/e2e/provisioner/provisioner_test.go | 25 ++ 8 files changed, 449 insertions(+), 122 deletions(-) create mode 100644 changelogs/unreleased/6147-sunjayBhatia-small.md diff --git a/changelogs/unreleased/6147-sunjayBhatia-small.md b/changelogs/unreleased/6147-sunjayBhatia-small.md new file mode 100644 index 00000000000..3ab0d2cf58a --- /dev/null +++ b/changelogs/unreleased/6147-sunjayBhatia-small.md @@ -0,0 +1 @@ +Gateway API provisioner now checks `gateway.networking.k8s.io/bundle-version` annotation on Gateway CRDs and sets SupportedVersion status condition on GatewayClass if annotation value matches supported Gateway API version. Best-effort support is provided if version does not match. diff --git a/examples/gateway-provisioner/01-roles.yaml b/examples/gateway-provisioner/01-roles.yaml index b43bba73e28..32bafdae071 100644 --- a/examples/gateway-provisioner/01-roles.yaml +++ b/examples/gateway-provisioner/01-roles.yaml @@ -39,6 +39,14 @@ rules: - list - update - watch +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - get + - list + - watch - apiGroups: - apps resources: diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index 08760885e9d..25c052c5da0 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -19830,6 +19830,14 @@ rules: - list - update - watch +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - get + - list + - watch - apiGroups: - apps resources: diff --git a/internal/provisioner/controller/gatewayclass.go b/internal/provisioner/controller/gatewayclass.go index f452393fbb6..69b96b131cb 100644 --- a/internal/provisioner/controller/gatewayclass.go +++ b/internal/provisioner/controller/gatewayclass.go @@ -22,6 +22,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + apiextensions_v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -37,6 +38,11 @@ import ( gatewayapi_v1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) +const ( + gatewayAPIBundleVersionAnnotation = "gateway.networking.k8s.io/bundle-version" + gatewayAPICRDBundleSupportedVersion = "v1.0.0" +) + // gatewayClassReconciler reconciles GatewayClass objects. type gatewayClassReconciler struct { gatewayController gatewayapi_v1beta1.GatewayController @@ -141,19 +147,25 @@ func (r *gatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, nil } + // Collect various status conditions here so we can update using + // setConditions. + statusConditions := map[string]metav1.Condition{} + + statusConditions[string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion)] = r.getSupportedVersionCondition(ctx) + ok, params, err := r.isValidParametersRef(ctx, gatewayClass.Spec.ParametersRef) if err != nil { return ctrl.Result{}, fmt.Errorf("error checking gateway class's parametersRef: %w", err) } if !ok { - if err := r.setAcceptedCondition( - ctx, - gatewayClass, - metav1.ConditionFalse, - gatewayapi_v1.GatewayClassReasonInvalidParameters, - "Invalid ParametersRef, must be a reference to an existing namespaced projectcontour.io/ContourDeployment resource", - ); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set gateway class %s Accepted condition: %w", req, err) + statusConditions[string(gatewayapi_v1.GatewayClassConditionStatusAccepted)] = metav1.Condition{ + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + Message: "Invalid ParametersRef, must be a reference to an existing namespaced projectcontour.io/ContourDeployment resource", + } + if err := r.setConditions(ctx, gatewayClass, statusConditions); err != nil { + return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -227,70 +239,103 @@ func (r *gatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request } if len(invalidParamsMessages) > 0 { - if err := r.setAcceptedCondition( - ctx, - gatewayClass, - metav1.ConditionFalse, - gatewayapi_v1.GatewayClassReasonInvalidParameters, - strings.Join(invalidParamsMessages, "; "), - ); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set gateway class %s Accepted condition: %w", req, err) + statusConditions[string(gatewayapi_v1.GatewayClassConditionStatusAccepted)] = metav1.Condition{ + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + Message: strings.Join(invalidParamsMessages, "; "), + } + if err := r.setConditions(ctx, gatewayClass, statusConditions); err != nil { + return ctrl.Result{}, err } return ctrl.Result{}, nil } } - if err := r.setAcceptedCondition(ctx, gatewayClass, metav1.ConditionTrue, gatewayapi_v1.GatewayClassReasonAccepted, "GatewayClass has been accepted by the controller"); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set gateway class %s Accepted condition: %w", req, err) + statusConditions[string(gatewayapi_v1.GatewayClassConditionStatusAccepted)] = metav1.Condition{ + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + Message: "GatewayClass has been accepted by the controller", + } + if err := r.setConditions(ctx, gatewayClass, statusConditions); err != nil { + return ctrl.Result{}, err } return ctrl.Result{}, nil } -func (r *gatewayClassReconciler) setAcceptedCondition( - ctx context.Context, - gatewayClass *gatewayapi_v1beta1.GatewayClass, - status metav1.ConditionStatus, - reason gatewayapi_v1beta1.GatewayClassConditionReason, - message string, -) error { - currentAcceptedCondition := metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: status, - ObservedGeneration: gatewayClass.Generation, - LastTransitionTime: metav1.Now(), - Reason: string(reason), - Message: message, - } - var newConds []metav1.Condition - for _, cond := range gatewayClass.Status.Conditions { - if cond.Type == string(gatewayapi_v1.GatewayClassConditionStatusAccepted) { - if cond.Status == status { +func (r *gatewayClassReconciler) setConditions(ctx context.Context, gatewayClass *gatewayapi_v1beta1.GatewayClass, newConds map[string]metav1.Condition) error { + var unchangedConds, updatedConds []metav1.Condition + for _, existing := range gatewayClass.Status.Conditions { + if cond, ok := newConds[existing.Type]; ok { + if existing.Status == cond.Status { // If status hasn't changed, don't change the condition, just // update the generation. - currentAcceptedCondition = cond - currentAcceptedCondition.ObservedGeneration = gatewayClass.Generation + changed := existing + changed.ObservedGeneration = gatewayClass.Generation + updatedConds = append(updatedConds, changed) + delete(newConds, cond.Type) } - - continue + } else { + unchangedConds = append(unchangedConds, existing) } - - newConds = append(newConds, cond) } - r.log.WithValues("gatewayclass-name", gatewayClass.Name).Info(fmt.Sprintf("setting gateway class's Accepted condition to %s", status)) + transitionTime := metav1.Now() + for _, c := range newConds { + r.log.WithValues("gatewayclass-name", gatewayClass.Name).Info(fmt.Sprintf("setting gateway class's %s condition to %s", c.Type, c.Status)) + c.ObservedGeneration = gatewayClass.Generation + c.LastTransitionTime = transitionTime + updatedConds = append(updatedConds, c) + } // nolint:gocritic - gatewayClass.Status.Conditions = append(newConds, currentAcceptedCondition) + gatewayClass.Status.Conditions = append(unchangedConds, updatedConds...) if err := r.client.Status().Update(ctx, gatewayClass); err != nil { - return fmt.Errorf("failed to set gatewayclass %s accepted condition: %w", gatewayClass.Name, err) + return fmt.Errorf("failed to update gateway class %s status: %w", gatewayClass.Name, err) } - return nil } +func (r *gatewayClassReconciler) getSupportedVersionCondition(ctx context.Context) metav1.Condition { + cond := metav1.Condition{ + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + // Assume false until we get to the happy case. + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonUnsupportedVersion), + } + gatewayClassCRD := &apiextensions_v1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclasses." + gatewayapi_v1.GroupName, + }, + } + if err := r.client.Get(ctx, client.ObjectKeyFromObject(gatewayClassCRD), gatewayClassCRD); err != nil { + errorMsg := "Error fetching gatewayclass CRD resource to validate supported version" + r.log.Error(err, errorMsg) + cond.Message = fmt.Sprintf("%s: %s. Resources will be reconciled with best-effort.", errorMsg, err) + return cond + } + + version, ok := gatewayClassCRD.Annotations[gatewayAPIBundleVersionAnnotation] + if !ok { + cond.Message = fmt.Sprintf("Bundle version annotation %s not found. Resources will be reconciled with best-effort.", gatewayAPIBundleVersionAnnotation) + return cond + } + if version != gatewayAPICRDBundleSupportedVersion { + cond.Message = fmt.Sprintf("Gateway API CRD bundle version %s is not supported. Resources will be reconciled with best-effort.", version) + return cond + } + + // No errors found, we can return true. + cond.Status = metav1.ConditionTrue + cond.Reason = string(gatewayapi_v1.GatewayClassReasonSupportedVersion) + cond.Message = fmt.Sprintf("Gateway API CRD bundle version %s is supported.", gatewayAPICRDBundleSupportedVersion) + return cond +} + // isValidParametersRef returns true if the provided ParametersReference is // to a ContourDeployment resource that exists. func (r *gatewayClassReconciler) isValidParametersRef(ctx context.Context, ref *gatewayapi_v1beta1.ParametersReference) (bool, *contour_api_v1alpha1.ContourDeployment, error) { diff --git a/internal/provisioner/controller/gatewayclass_test.go b/internal/provisioner/controller/gatewayclass_test.go index ac302f1971c..596732e84ac 100644 --- a/internal/provisioner/controller/gatewayclass_test.go +++ b/internal/provisioner/controller/gatewayclass_test.go @@ -15,20 +15,24 @@ package controller import ( "context" + "sort" "testing" + "github.com/bombsimon/logrusr/v4" contourv1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" + "github.com/projectcontour/contour/internal/fixture" "github.com/projectcontour/contour/internal/provisioner" "github.com/projectcontour/contour/internal/ref" - - "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + apiextensions_v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -36,11 +40,12 @@ import ( func TestGatewayClassReconcile(t *testing.T) { tests := map[string]struct { - gatewayClass *gatewayv1beta1.GatewayClass - params *contourv1alpha1.ContourDeployment - req *reconcile.Request - wantCondition *metav1.Condition - assertions func(t *testing.T, r *gatewayClassReconciler, gc *gatewayv1beta1.GatewayClass, reconcileErr error) + gatewayClass *gatewayv1beta1.GatewayClass + gatewayClassCRD *apiextensions_v1.CustomResourceDefinition + params *contourv1alpha1.ContourDeployment + req *reconcile.Request + wantConditions []*metav1.Condition + assertions func(t *testing.T, r *gatewayClassReconciler, gc *gatewayv1beta1.GatewayClass, reconcileErr error) }{ "reconcile request for non-existent gatewayclass results in no error": { req: &reconcile.Request{ @@ -72,7 +77,7 @@ func TestGatewayClassReconcile(t *testing.T) { assert.Empty(t, res.Status.Conditions) }, }, - "gatewayclass controlled by us with no parameters gets Accepted: true condition": { + "gatewayclass controlled by us with no parameters gets Accepted: true condition and SupportedVersion: true": { gatewayClass: &gatewayv1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ Name: "gatewayclass-1", @@ -81,10 +86,17 @@ func TestGatewayClassReconcile(t *testing.T) { ControllerName: "projectcontour.io/gateway-controller", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with an invalid parametersRef (target does not exist) gets Accepted: false condition": { @@ -102,10 +114,17 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with an invalid parametersRef (invalid group) gets Accepted: false condition": { @@ -129,10 +148,17 @@ func TestGatewayClassReconcile(t *testing.T) { Name: "gatewayclass-params", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with an invalid parametersRef (invalid kind) gets Accepted: false condition": { @@ -156,10 +182,17 @@ func TestGatewayClassReconcile(t *testing.T) { Name: "gatewayclass-params", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with an invalid parametersRef (invalid name) gets Accepted: false condition": { @@ -183,10 +216,17 @@ func TestGatewayClassReconcile(t *testing.T) { Name: "gatewayclass-params", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with an invalid parametersRef (invalid namespace) gets Accepted: false condition": { @@ -210,10 +250,17 @@ func TestGatewayClassReconcile(t *testing.T) { Name: "gatewayclass-params", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef gets Accepted: true condition": { @@ -237,10 +284,17 @@ func TestGatewayClassReconcile(t *testing.T) { Name: "gatewayclass-params", }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef but invalid parameter values for NetworkPublishing gets Accepted: false condition": { @@ -272,10 +326,17 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef but invalid parameter values for ExtraVolumeMounts gets Accepted: false condition": { @@ -313,10 +374,17 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef but invalid parameter values for LogLevel gets Accepted: false condition": { @@ -345,10 +413,17 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef but invalid parameter values for ExternalTrafficPolicy gets Accepted: false condition": { @@ -379,10 +454,17 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, "gatewayclass controlled by us with a valid parametersRef but invalid parameter values for IPFamilyPolicy gets Accepted: false condition": { @@ -413,13 +495,121 @@ func TestGatewayClassReconcile(t *testing.T) { }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonInvalidParameters), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + }, }, }, - "gatewayclass with status from previous generation is updated": { + "gatewayclass controlled by us with gatewayclass CRD with unsupported version sets Accepted: true, SupportedVersion: False": { + gatewayClass: &gatewayv1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclass-1", + }, + Spec: gatewayv1beta1.GatewayClassSpec{ + ControllerName: "projectcontour.io/gateway-controller", + }, + }, + gatewayClassCRD: &apiextensions_v1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclasses.gateway.networking.k8s.io", + Annotations: map[string]string{ + "gateway.networking.k8s.io/bundle-version": "v9.9.9", + }, + }, + }, + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonUnsupportedVersion), + }, + }, + assertions: func(t *testing.T, r *gatewayClassReconciler, gc *gatewayv1beta1.GatewayClass, reconcileErr error) { + require.NoError(t, reconcileErr) + }, + }, + "gatewayclass controlled by us with gatewayclass CRD fetch failed sets SupportedVersion: false": { + gatewayClass: &gatewayv1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclass-1", + }, + Spec: gatewayv1beta1.GatewayClassSpec{ + ControllerName: "projectcontour.io/gateway-controller", + }, + }, + gatewayClassCRD: &apiextensions_v1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + // Use the wrong name so we fail to fetch the CRD, + // contrived way to cause this scenario. + Name: "gatewayclasses-wrong.gateway.networking.k8s.io", + Annotations: map[string]string{ + "gateway.networking.k8s.io/bundle-version": "v1.0.0", + }, + }, + }, + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonUnsupportedVersion), + }, + }, + assertions: func(t *testing.T, r *gatewayClassReconciler, gc *gatewayv1beta1.GatewayClass, reconcileErr error) { + require.NoError(t, reconcileErr) + }, + }, + "gatewayclass controlled by us with gatewayclass CRD without version annotation sets SupportedVersion: false": { + gatewayClass: &gatewayv1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclass-1", + }, + Spec: gatewayv1beta1.GatewayClassSpec{ + ControllerName: "projectcontour.io/gateway-controller", + }, + }, + gatewayClassCRD: &apiextensions_v1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclasses.gateway.networking.k8s.io", + Annotations: map[string]string{ + "gateway.networking.k8s.io/bundle-version-wrong": "v1.0.0", + }, + }, + }, + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayClassReasonUnsupportedVersion), + }, + }, + assertions: func(t *testing.T, r *gatewayClassReconciler, gc *gatewayv1beta1.GatewayClass, reconcileErr error) { + require.NoError(t, reconcileErr) + }, + }, + "gatewayclass with status from previous generation is updated, only conditions we own are changed": { gatewayClass: &gatewayv1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ Name: "gatewayclass-1", @@ -429,19 +619,41 @@ func TestGatewayClassReconcile(t *testing.T) { ControllerName: "projectcontour.io/gateway-controller", }, Status: gatewayv1beta1.GatewayClassStatus{ - Conditions: []metav1.Condition{{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), - ObservedGeneration: 1, - }}, + Conditions: []metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + ObservedGeneration: 1, + }, + { + Type: "SomeOtherCondition", + Status: metav1.ConditionTrue, + Reason: "FooReason", + ObservedGeneration: 1, + }, + }, }, }, - wantCondition: &metav1.Condition{ - Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), - ObservedGeneration: 2, + wantConditions: []*metav1.Condition{ + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonAccepted), + ObservedGeneration: 2, + }, + { + Type: string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion), + Status: metav1.ConditionTrue, + Reason: string(gatewayapi_v1.GatewayClassReasonSupportedVersion), + ObservedGeneration: 2, + }, + { + Type: "SomeOtherCondition", + Status: metav1.ConditionTrue, + Reason: "FooReason", + ObservedGeneration: 1, + }, }, }, } @@ -456,14 +668,29 @@ func TestGatewayClassReconcile(t *testing.T) { client.WithObjects(tc.gatewayClass) client.WithStatusSubresource(tc.gatewayClass) } + + if tc.gatewayClassCRD != nil { + client.WithObjects(tc.gatewayClassCRD) + } else { + client.WithObjects(&apiextensions_v1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gatewayclasses.gateway.networking.k8s.io", + Annotations: map[string]string{ + "gateway.networking.k8s.io/bundle-version": "v1.0.0", + }, + }, + }) + } + if tc.params != nil { client.WithObjects(tc.params) } + log.SetLogger(logrusr.New(fixture.NewTestLogger(t))) r := &gatewayClassReconciler{ gatewayController: "projectcontour.io/gateway-controller", client: client.Build(), - log: logr.Discard(), + log: ctrl.Log.WithName("gatewayclass-controller-test"), } var req reconcile.Request @@ -477,15 +704,25 @@ func TestGatewayClassReconcile(t *testing.T) { _, err = r.Reconcile(context.Background(), req) - if tc.wantCondition != nil { + if len(tc.wantConditions) > 0 { res := &gatewayv1beta1.GatewayClass{} require.NoError(t, r.client.Get(context.Background(), keyFor(tc.gatewayClass), res)) - require.Len(t, res.Status.Conditions, 1) - assert.Equal(t, tc.wantCondition.Type, res.Status.Conditions[0].Type) - assert.Equal(t, tc.wantCondition.Status, res.Status.Conditions[0].Status) - assert.Equal(t, tc.wantCondition.Reason, res.Status.Conditions[0].Reason) - assert.Equal(t, tc.wantCondition.ObservedGeneration, res.Status.Conditions[0].ObservedGeneration) + require.Len(t, res.Status.Conditions, len(tc.wantConditions)) + + sort.Slice(tc.wantConditions, func(i, j int) bool { + return tc.wantConditions[i].Type < tc.wantConditions[j].Type + }) + sort.Slice(res.Status.Conditions, func(i, j int) bool { + return res.Status.Conditions[i].Type < res.Status.Conditions[j].Type + }) + + for i := range tc.wantConditions { + assert.Equal(t, tc.wantConditions[i].Type, res.Status.Conditions[i].Type) + assert.Equal(t, tc.wantConditions[i].Status, res.Status.Conditions[i].Status) + assert.Equal(t, tc.wantConditions[i].Reason, res.Status.Conditions[i].Reason) + assert.Equal(t, tc.wantConditions[i].ObservedGeneration, res.Status.Conditions[i].ObservedGeneration) + } } if tc.assertions != nil { diff --git a/internal/provisioner/rbac/rbac.go b/internal/provisioner/rbac/rbac.go index 1f74499318e..588684f3483 100644 --- a/internal/provisioner/rbac/rbac.go +++ b/internal/provisioner/rbac/rbac.go @@ -22,6 +22,7 @@ package rbac // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses;gateways,verbs=get;list;watch // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses/status;gateways/status,verbs=update // +kubebuilder:rbac:groups=projectcontour.io,resources=contourdeployments,verbs=get;list;watch +// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch // --- // RBAC for core Contour resources to be provisioned. diff --git a/internal/provisioner/scheme.go b/internal/provisioner/scheme.go index b1f8c305bba..367a1edba0a 100644 --- a/internal/provisioner/scheme.go +++ b/internal/provisioner/scheme.go @@ -15,6 +15,7 @@ package provisioner import ( contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" + apiextensions_v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" gateway_api_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -28,6 +29,7 @@ func CreateScheme() (*runtime.Scheme, error) { b := runtime.SchemeBuilder{ clientgoscheme.AddToScheme, + apiextensions_v1.AddToScheme, gateway_api_v1alpha2.AddToScheme, gateway_api_v1beta1.AddToScheme, contour_api_v1alpha1.AddToScheme, diff --git a/test/e2e/provisioner/provisioner_test.go b/test/e2e/provisioner/provisioner_test.go index 5f822287edd..f1c24740885 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" + "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" gatewayapi_v1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -139,6 +140,28 @@ var _ = AfterSuite(func() { }) var _ = Describe("Gateway provisioner", func() { + Specify("GatewayClass status condition SupportedVersion is set to True", func() { + // This test will fail if we bump the Gateway API module and CRDs but + // forget to update the supported version we check for. + require.Eventually(f.T(), func() bool { + gc := &gatewayapi_v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + }, + } + if err := f.Client.Get(context.TODO(), client.ObjectKeyFromObject(gc), gc); err != nil { + return false + } + for _, cond := range gc.Status.Conditions { + if cond.Type == string(gatewayapi_v1.GatewayClassConditionStatusSupportedVersion) && + cond.Status == metav1.ConditionTrue { + return true + } + } + return false + }, f.RetryTimeout, f.RetryInterval) + }) + f.NamespacedTest("provisioner-gatewayclass-params", func(namespace string) { Specify("GatewayClass parameters are handled correctly", func() { // Create GatewayClass with a reference to a nonexistent ContourDeployment, @@ -229,6 +252,7 @@ var _ = Describe("Gateway provisioner", func() { require.NoError(f.T(), f.DeleteGatewayClass(gatewayClass, false)) }) }) + f.NamespacedTest("gateway-with-envoy-deployment", func(namespace string) { Specify("A gateway with Envoy as a deployment can be provisioned and routes traffic correctly", func() { gateway := &gatewayapi_v1beta1.Gateway{ @@ -297,6 +321,7 @@ var _ = Describe("Gateway provisioner", func() { assert.Equal(f.T(), "echo", body.Service) }) }) + f.NamespacedTest("gateway-with-many-listeners", func(namespace string) { Specify("A gateway with many Listeners for different protocols can be provisioned and routes correctly", func() { f.Certs.CreateSelfSignedCert(namespace, "https-1-cert", "https-1-cert", "https-1.provisioner.projectcontour.io")