Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add PolicyStatus for EnvoyPatchPolicy #2910

Merged
merged 5 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions api/v1alpha1/envoypatchpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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"

Expand Down
22 changes: 0 additions & 22 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Large diffs are not rendered by default.

50 changes: 1 addition & 49 deletions internal/cmd/egctl/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will update egctl x status as a follow-up

}

for _, tc := range testCases {
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/egctl/translate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestTranslate(t *testing.T) {
to: "xds",
output: yamlOutput,
resourceType: string(AllEnvoyConfigType),
expect: true,
expect: false,
},
{
name: "default-resources",
Expand Down
113 changes: 74 additions & 39 deletions internal/gatewayapi/envoypatchpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
"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"
Expand All @@ -26,21 +26,43 @@
})

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))

Check warning on line 40 in internal/gatewayapi/envoypatchpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/envoypatchpolicy.go#L40

Added line #L40 was not covered by tests
}

// 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]
Expand All @@ -56,40 +78,58 @@

// 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
}

Expand All @@ -107,11 +147,6 @@
}

// Set Accepted=True
status.SetEnvoyPatchPolicyCondition(policy,
gwv1a2.PolicyConditionAccepted,
metav1.ConditionTrue,
gwv1a2.PolicyReasonAccepted,
"EnvoyPatchPolicy has been accepted.",
)
status.SetAcceptedForPolicyAncestors(&policy.Status, ancestorRefs, t.GatewayControllerName)
}
}
1 change: 1 addition & 0 deletions internal/gatewayapi/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,20 @@ xdsIR:
- name: edit-conn-buffer-bytes
namespace: envoy-gateway
status:
conditions:
- lastTransitionTime: null
message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:Gateway,
only TargetRef.Group:gateway.networking.k8s.io and TargetRef.Kind:GatewayClass
is supported.
reason: Invalid
status: "False"
type: Accepted
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: GatewayClass
name: gateway-1
conditions:
- lastTransitionTime: null
message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:Gateway,
only TargetRef.Group:gateway.networking.k8s.io and TargetRef.Kind:GatewayClass
is supported.
reason: Invalid
status: "False"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
http:
- address: 0.0.0.0
hostnames:
Expand Down
Loading
Loading