comment and test for edge case

Signed-off-by: Steve Kriss <krisss@vmware.com>
projectcontour · Apr 12, 2022 · b0c7b27 · b0c7b27
1 parent 3c3be71
commit b0c7b27
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 0 deletions.
diff --git a/internal/provisioner/controller/gateway.go b/internal/provisioner/controller/gateway.go
@@ -334,6 +334,15 @@ func (r *gatewayReconciler) ensureContourDeleted(ctx context.Context, contour *m
 }
 
 func (r *gatewayReconciler) getGatewayClassParams(ctx context.Context, gatewayClass *gatewayapi_v1alpha2.GatewayClass) (*contour_api_v1alpha1.ContourDeployment, error) {
+	// Check if there is a parametersRef to ContourDeployment with
+	// a namespace specified. Theoretically, we should only be reconciling
+	// Gateways for GatewayClasses that have valid parameter refs (or no refs),
+	// making this check mostly redundant other than checking for a nil params
+	// ref, but there is potentially a race condition where a GatewayClass's
+	// parameters ref is updated from valid to invalid, and then a Gateway reconcile
+	// is triggered before the GatewayClass's status is updated, that
+	// would lead to this code being executed for a GatewayClass with an
+	// invalid parametersRef.
 	if !isContourDeploymentRef(gatewayClass.Spec.ParametersRef) {
 		return nil, nil
 	}

diff --git a/internal/provisioner/controller/gateway_test.go b/internal/provisioner/controller/gateway_test.go
@@ -69,6 +69,17 @@ func TestGatewayReconcile(t *testing.T) {
 		return gc
 	}
 
+	reconcilableGatewayClassWithInvalidParams := func(name, controller string) *gatewayv1alpha2.GatewayClass {
+		gc := reconcilableGatewayClass(name, controller)
+		gc.Spec.ParametersRef = &gatewayv1alpha2.ParametersReference{
+			Group:     gatewayv1alpha2.Group(contourv1alpha1.GroupVersion.Group),
+			Kind:      "InvalidKind",
+			Namespace: gatewayapi.NamespacePtr("projectcontour"),
+			Name:      name + "-params",
+		}
+		return gc
+	}
+
 	tests := map[string]struct {
 		gatewayClass       *gatewayv1alpha2.GatewayClass
 		gatewayClassParams *contourv1alpha1.ContourDeployment
@@ -464,6 +475,69 @@ func TestGatewayReconcile(t *testing.T) {
 					},
 				}
 
+				assert.Equal(t, want, contourConfig.Spec)
+			},
+		},
+		"If the Gateway's GatewayClass parametersRef is invalid it's ignored and the Gateway gets a default ContourConfiguration": {
+			gatewayClass: reconcilableGatewayClassWithInvalidParams("gatewayclass-1", controller),
+			gatewayClassParams: &contourv1alpha1.ContourDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "projectcontour",
+					Name:      "gatewayclass-1-params",
+				},
+				Spec: contourv1alpha1.ContourDeploymentSpec{
+					Config: &contourv1alpha1.ContourConfigurationSpec{
+						EnableExternalNameService: pointer.Bool(true),
+						Envoy: &contourv1alpha1.EnvoyConfig{
+							Listener: &contourv1alpha1.EnvoyListenerConfig{
+								DisableMergeSlashes: pointer.Bool(true),
+							},
+						},
+					},
+				},
+			},
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "gateway-1",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: gatewayv1alpha2.ObjectName("gatewayclass-1"),
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+
+				// Verify the Gateway has a "Scheduled: true" condition
+				require.NoError(t, r.client.Get(context.Background(), keyFor(gw), gw))
+				require.Len(t, gw.Status.Conditions, 1)
+				assert.Equal(t, string(gatewayv1alpha2.GatewayConditionScheduled), gw.Status.Conditions[0].Type)
+				assert.Equal(t, metav1.ConditionTrue, gw.Status.Conditions[0].Status)
+
+				// Verify the ContourConfiguration has been created
+				contourConfig := &contourv1alpha1.ContourConfiguration{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "gateway-1",
+						Name:      "contourconfig-gateway-1",
+					},
+				}
+				require.NoError(t, r.client.Get(context.Background(), keyFor(contourConfig), contourConfig))
+
+				want := contourv1alpha1.ContourConfigurationSpec{
+					Gateway: &contourv1alpha1.GatewayConfig{
+						GatewayRef: &contourv1alpha1.NamespacedName{
+							Namespace: gw.Name,
+							Name:      gw.Name,
+						},
+					},
+					Envoy: &contourv1alpha1.EnvoyConfig{
+						Service: &contourv1alpha1.NamespacedName{
+							Namespace: gw.Namespace,
+							Name:      "envoy-" + gw.Name,
+						},
+					},
+				}
+
 				assert.Equal(t, want, contourConfig.Spec)
 			},
 		},