Merge branch 'main' into eg_startup_reliability

api/v1alpha1/envoypatchpolicy_types.go

@@ -32,7 +32,7 @@ type EnvoyPatchPolicy struct {
 	Spec EnvoyPatchPolicySpec `json:"spec"`
 
 	// Status defines the current status of EnvoyPatchPolicy.
-	Status EnvoyPatchPolicyStatus `json:"status,omitempty"`
+	Status gwapiv1a2.PolicyStatus `json:"status,omitempty"`
 }
 
 // EnvoyPatchPolicySpec defines the desired state of EnvoyPatchPolicy.
@@ -123,17 +123,6 @@ type JSONPatchOperation struct {
 	Value *apiextensionsv1.JSON `json:"value,omitempty"`
 }
 
-// EnvoyPatchPolicyStatus defines the state of EnvoyPatchPolicy
-type EnvoyPatchPolicyStatus struct {
-	// Conditions describe the current conditions of the EnvoyPatchPolicy.
-	//
-	// +optional
-	// +listType=map
-	// +listMapKey=type
-	// +kubebuilder:validation:MaxItems=8
-	Conditions []metav1.Condition `json:"conditions,omitempty"`
-}
-
 const (
 	// PolicyConditionProgrammed indicates whether the policy has been translated
 	// and ready to be programmed into the data plane.
@@ -157,7 +146,7 @@ const (
 	// is syntactically or semantically invalid.
 	PolicyReasonInvalid gwapiv1a2.PolicyConditionReason = "Invalid"
 
-	// PolicyReasonTargetNotFound is used with the "Programmed" condition when the
+	// PolicyReasonResourceNotFound is used with the "Programmed" condition when the
 	// policy cannot find the resource type to patch to.
 	PolicyReasonResourceNotFound gwapiv1a2.PolicyConditionReason = "ResourceNotFound"
 

go.mod

@@ -4,6 +4,7 @@ go 1.21
 
 require (
 	fortio.org/fortio v1.63.4
+	fortio.org/log v1.12.0
 	github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa
 	github.com/davecgh/go-spew v1.1.1
 	github.com/envoyproxy/go-control-plane v0.12.0
@@ -53,7 +54,6 @@ require (
 
 require (
 	fortio.org/dflag v1.7.0 // indirect
-	fortio.org/log v1.12.0 // indirect
 	fortio.org/sets v1.0.3 // indirect
 	fortio.org/struct2env v0.4.0 // indirect
 	fortio.org/version v1.0.3 // indirect

internal/cmd/egctl/status_test.go

@@ -15,8 +15,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
-
-	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 )
 
 func TestWriteStatus(t *testing.T) {
@@ -518,53 +516,7 @@ btls      foobar2   test-status-2   test reason 2
 `,
 			expect: true,
 		},
-		{
-			name: "egctl x status envoypatchpolicy with typed name",
-			resourceList: &egv1a1.EnvoyPatchPolicyList{
-				Items: []egv1a1.EnvoyPatchPolicy{
-					{
-						TypeMeta: metav1.TypeMeta{
-							Kind: "EnvoyPatchPolicy",
-						},
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      "epp",
-							Namespace: "default",
-						},
-						Status: egv1a1.EnvoyPatchPolicyStatus{
-							Conditions: []metav1.Condition{
-								{
-									Type:               "foobar1",
-									Status:             metav1.ConditionStatus("test-status-1"),
-									ObservedGeneration: 123456,
-									LastTransitionTime: metav1.NewTime(testTime),
-									Reason:             "test reason 1",
-									Message:            "test message 1",
-								},
-								{
-									Type:               "foobar2",
-									Status:             metav1.ConditionStatus("test-status-2"),
-									ObservedGeneration: 123457,
-									LastTransitionTime: metav1.NewTime(testTime.Add(1 * time.Hour)),
-									Reason:             "test reason 2",
-									Message:            "test message 2",
-								},
-							},
-						},
-					},
-				},
-			},
-			resourceNamespaced: true,
-			resourceType:       "envoypatchpolicy",
-			quiet:              false,
-			verbose:            false,
-			allNamespaces:      false,
-			typedName:          true,
-			outputs: `NAME                   TYPE      STATUS          REASON
-envoypatchpolicy/epp   foobar2   test-status-2   test reason 2
-                       foobar1   test-status-1   test reason 1
-`,
-			expect: true,
-		},
+		// TODO(sh2): add a policy status test for egctl x status cmd
 	}
 
 	for _, tc := range testCases {

internal/cmd/egctl/translate_test.go

@@ -183,7 +183,7 @@ func TestTranslate(t *testing.T) {
 			to:           "xds",
 			output:       yamlOutput,
 			resourceType: string(AllEnvoyConfigType),
-			expect:       true,
+			expect:       false,
 		},
 		{
 			name:      "default-resources",

internal/gatewayapi/envoypatchpolicy.go

@@ -9,9 +9,9 @@ import (
 	"fmt"
 	"sort"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
-	gwv1b1 "sigs.k8s.io/gateway-api/apis/v1"
+	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -26,21 +26,43 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 	})
 
 	for _, policy := range envoyPatchPolicies {
-		policy := policy.DeepCopy()
-		targetNs := policy.Spec.TargetRef.Namespace
-		targetKind := KindGateway
+		var (
+			policy       = policy.DeepCopy()
+			ancestorRefs []gwv1a2.ParentReference
+			resolveErr   *status.PolicyResolveError
+			targetKind   string
+			irKey        string
+		)
 
+		targetNs := policy.Spec.TargetRef.Namespace
 		// If empty, default to namespace of policy
 		if targetNs == nil {
-			targetNs = ptr.To(gwv1b1.Namespace(policy.Namespace))
+			targetNs = ptr.To(gwv1.Namespace(policy.Namespace))
 		}
 
-		// Get the IR
-		// It must exist since the gateways have already been processed
-		irKey := irStringKey(string(*targetNs), string(policy.Spec.TargetRef.Name))
 		if t.MergeGateways {
-			irKey = string(t.GatewayClassName)
 			targetKind = KindGatewayClass
+			irKey = string(t.GatewayClassName)
+
+			ancestorRefs = []gwv1a2.ParentReference{
+				{
+					Group: GroupPtr(gwv1.GroupName),
+					Kind:  KindPtr(targetKind),
+					Name:  policy.Spec.TargetRef.Name,
+				},
+			}
+		} else {
+			targetKind = KindGateway
+			gatewayNN := types.NamespacedName{
+				Namespace: string(*targetNs),
+				Name:      string(policy.Spec.TargetRef.Name),
+			}
+			// It must exist since the gateways have already been processed
+			irKey = irStringKey(gatewayNN.Namespace, gatewayNN.Name)
+
+			ancestorRefs = []gwv1a2.ParentReference{
+				getAncestorRefForPolicy(gatewayNN, nil),
+			}
 		}
 
 		gwXdsIR, ok := xdsIR[irKey]
@@ -56,40 +78,58 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 
 		// Append the IR
 		gwXdsIR.EnvoyPatchPolicies = append(gwXdsIR.EnvoyPatchPolicies, &policyIR)
-		if policy.Spec.TargetRef.Group != gwv1b1.GroupName || string(policy.Spec.TargetRef.Kind) != targetKind {
-			message := fmt.Sprintf("TargetRef.Group:%s TargetRef.Kind:%s, only TargetRef.Group:%s and TargetRef.Kind:%s is supported.",
-				policy.Spec.TargetRef.Group, policy.Spec.TargetRef.Kind, gwv1b1.GroupName, targetKind)
 
-			status.SetEnvoyPatchPolicyCondition(policy,
-				gwv1a2.PolicyConditionAccepted,
-				metav1.ConditionFalse,
-				gwv1a2.PolicyReasonInvalid,
-				message,
+		// Ensure EnvoyPatchPolicy is enabled
+		if !t.EnvoyPatchPolicyEnabled {
+			resolveErr = &status.PolicyResolveError{
+				Reason:  egv1a1.PolicyReasonDisabled,
+				Message: "EnvoyPatchPolicy is disabled in the EnvoyGateway configuration",
+			}
+			status.SetResolveErrorForPolicyAncestors(&policy.Status,
+				ancestorRefs,
+				t.GatewayControllerName,
+				policy.Generation,
+				resolveErr,
+			)
+
+			continue
+		}
+
+		// Ensure EnvoyPatchPolicy is targeting to a support type
+		if policy.Spec.TargetRef.Group != gwv1.GroupName || string(policy.Spec.TargetRef.Kind) != targetKind {
+			message := fmt.Sprintf("TargetRef.Group:%s TargetRef.Kind:%s, only TargetRef.Group:%s and TargetRef.Kind:%s is supported.",
+				policy.Spec.TargetRef.Group, policy.Spec.TargetRef.Kind, gwv1.GroupName, targetKind)
+
+			resolveErr = &status.PolicyResolveError{
+				Reason:  gwv1a2.PolicyReasonInvalid,
+				Message: message,
+			}
+			status.SetResolveErrorForPolicyAncestors(&policy.Status,
+				ancestorRefs,
+				t.GatewayControllerName,
+				policy.Generation,
+				resolveErr,
 			)
+
 			continue
 		}
 
-		// Ensure Policy and target Gateway are in the same namespace
+		// Ensure EnvoyPatchPolicy and target Gateway are in the same namespace
 		if policy.Namespace != string(*targetNs) {
 			message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, EnvoyPatchPolicy can only target a %s in the same namespace.",
 				policy.Namespace, *targetNs, targetKind)
 
-			status.SetEnvoyPatchPolicyCondition(policy,
-				gwv1a2.PolicyConditionAccepted,
-				metav1.ConditionFalse,
-				gwv1a2.PolicyReasonInvalid,
-				message,
+			resolveErr = &status.PolicyResolveError{
+				Reason:  gwv1a2.PolicyReasonInvalid,
+				Message: message,
+			}
+			status.SetResolveErrorForPolicyAncestors(&policy.Status,
+				ancestorRefs,
+				t.GatewayControllerName,
+				policy.Generation,
+				resolveErr,
 			)
-			continue
-		}
 
-		if !t.EnvoyPatchPolicyEnabled {
-			status.SetEnvoyPatchPolicyCondition(policy,
-				gwv1a2.PolicyConditionAccepted,
-				metav1.ConditionFalse,
-				egv1a1.PolicyReasonDisabled,
-				"EnvoyPatchPolicy is disabled in the EnvoyGateway configuration",
-			)
 			continue
 		}
 
@@ -107,11 +147,6 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 		}
 
 		// Set Accepted=True
-		status.SetEnvoyPatchPolicyCondition(policy,
-			gwv1a2.PolicyConditionAccepted,
-			metav1.ConditionTrue,
-			gwv1a2.PolicyReasonAccepted,
-			"EnvoyPatchPolicy has been accepted.",
-		)
+		status.SetAcceptedForPolicyAncestors(&policy.Status, ancestorRefs, t.GatewayControllerName)
 	}
 }

internal/gatewayapi/helpers.go

@@ -411,6 +411,7 @@ func protocolSliceToStringSlice(protocols []gwapiv1.ProtocolType) []string {
 	return protocolStrings
 }
 
+// getAncestorRefForPolicy returns Gateway as an ancestor reference for policy.
 func getAncestorRefForPolicy(gatewayNN types.NamespacedName, sectionName *v1alpha2.SectionName) v1alpha2.ParentReference {
 	return v1alpha2.ParentReference{
 		Group:       GroupPtr(gwapiv1.GroupName),

internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml

@@ -64,13 +64,20 @@ xdsIR:
     - name: edit-conn-buffer-bytes
       namespace: envoy-gateway-2
       status:
-        conditions:
-        - lastTransitionTime: null
-          message: Namespace:envoy-gateway-2 TargetRef.Namespace:envoy-gateway, EnvoyPatchPolicy
-            can only target a Gateway in the same namespace.
-          reason: Invalid
-          status: "False"
-          type: Accepted
+        ancestors:
+        - ancestorRef:
+            group: gateway.networking.k8s.io
+            kind: Gateway
+            name: gateway-1
+            namespace: envoy-gateway
+          conditions:
+          - lastTransitionTime: null
+            message: Namespace:envoy-gateway-2 TargetRef.Namespace:envoy-gateway,
+              EnvoyPatchPolicy can only target a Gateway in the same namespace.
+            reason: Invalid
+            status: "False"
+            type: Accepted
+          controllerName: gateway.envoyproxy.io/gatewayclass-controller
     http:
     - address: 0.0.0.0
       hostnames:

internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml

@@ -74,12 +74,18 @@ xdsIR:
     - name: edit-conn-buffer-bytes
       namespace: envoy-gateway
       status:
-        conditions:
-        - lastTransitionTime: null
-          message: EnvoyPatchPolicy is disabled in the EnvoyGateway configuration
-          reason: Disabled
-          status: "False"
-          type: Accepted
+        ancestors:
+        - ancestorRef:
+            group: gateway.networking.k8s.io
+            kind: GatewayClass
+            name: envoy-gateway-class
+          conditions:
+          - lastTransitionTime: null
+            message: EnvoyPatchPolicy is disabled in the EnvoyGateway configuration
+            reason: Disabled
+            status: "False"
+            type: Accepted
+          controllerName: gateway.envoyproxy.io/gatewayclass-controller
     http:
     - address: 0.0.0.0
       hostnames: