From c080b1c43e97c8cbb9504ea76030cde938b73688 Mon Sep 17 00:00:00 2001 From: Phil Brookes Date: Mon, 29 Apr 2024 10:28:48 +0200 Subject: [PATCH] propagate DNS Record status to DNS Policy (#585) --- api/v1alpha1/dnspolicy_types.go | 3 + api/v1alpha1/zz_generated.deepcopy.go | 18 +++ bundle/manifests/kuadrant.io_dnspolicies.yaml | 73 +++++++++ config/crd/bases/kuadrant.io_dnspolicies.yaml | 73 +++++++++ controllers/dnspolicy_status.go | 59 ++++++- controllers/dnspolicy_status_test.go | 151 ++++++++++++++++++ go.mod | 2 +- go.sum | 4 + 8 files changed, 374 insertions(+), 9 deletions(-) create mode 100644 controllers/dnspolicy_status_test.go diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index e3d60a0dc..6515b51ca 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -131,6 +131,9 @@ type DNSPolicyStatus struct { // +optional HealthCheck *HealthCheckStatus `json:"healthCheck,omitempty"` + + // +optional + ProbeConditions map[string][]metav1.Condition `json:"probeConditions,omitempty"` } func (s *DNSPolicyStatus) GetConditions() []metav1.Condition { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2af12eec3..214059290 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -187,6 +187,24 @@ func (in *DNSPolicyStatus) DeepCopyInto(out *DNSPolicyStatus) { *out = new(HealthCheckStatus) (*in).DeepCopyInto(*out) } + if in.ProbeConditions != nil { + in, out := &in.ProbeConditions, &out.ProbeConditions + *out = make(map[string][]v1.Condition, len(*in)) + for key, val := range *in { + var outVal []v1.Condition + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + (*out)[key] = outVal + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSPolicyStatus. diff --git a/bundle/manifests/kuadrant.io_dnspolicies.yaml b/bundle/manifests/kuadrant.io_dnspolicies.yaml index ade878b4c..37049204d 100644 --- a/bundle/manifests/kuadrant.io_dnspolicies.yaml +++ b/bundle/manifests/kuadrant.io_dnspolicies.yaml @@ -401,6 +401,79 @@ spec: recorded in the status condition format: int64 type: integer + probeConditions: + additionalProperties: + items: + description: "Condition contains details for one aspect of the + current state of this API Resource.\n---\nThis struct is intended + for direct use as an array at the field path .status.conditions. + \ For example,\n\n\n\ttype FooStatus struct{\n\t // Represents + the observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // + +listType=map\n\t // +listMapKey=type\n\t Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t // other + fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object type: object type: object served: true diff --git a/config/crd/bases/kuadrant.io_dnspolicies.yaml b/config/crd/bases/kuadrant.io_dnspolicies.yaml index 12b8b7f7d..d00229c30 100644 --- a/config/crd/bases/kuadrant.io_dnspolicies.yaml +++ b/config/crd/bases/kuadrant.io_dnspolicies.yaml @@ -400,6 +400,79 @@ spec: recorded in the status condition format: int64 type: integer + probeConditions: + additionalProperties: + items: + description: "Condition contains details for one aspect of the + current state of this API Resource.\n---\nThis struct is intended + for direct use as an array at the field path .status.conditions. + \ For example,\n\n\n\ttype FooStatus struct{\n\t // Represents + the observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // + +listType=map\n\t // +listMapKey=type\n\t Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t // other + fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object type: object type: object served: true diff --git a/controllers/dnspolicy_status.go b/controllers/dnspolicy_status.go index dd447aef7..27d53fdeb 100644 --- a/controllers/dnspolicy_status.go +++ b/controllers/dnspolicy_status.go @@ -20,6 +20,7 @@ import ( "context" "errors" "slices" + "strings" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -30,10 +31,13 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) +var NegativePolarityConditions []string + func (r *DNSPolicyReconciler) reconcileStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) (ctrl.Result, error) { newStatus := r.calculateStatus(ctx, dnsPolicy, specErr) @@ -76,19 +80,22 @@ func (r *DNSPolicyReconciler) calculateStatus(ctx context.Context, dnsPolicy *v1 return newStatus } - enforcedCondition := r.enforcedCondition(ctx, dnsPolicy) + recordsList := &kuadrantdnsv1alpha1.DNSRecordList{} + var enforcedCondition *metav1.Condition + if err := r.Client().List(ctx, recordsList); err != nil { + enforcedCondition = kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown(dnsPolicy.Kind(), err), false) + } else { + enforcedCondition = r.enforcedCondition(recordsList, dnsPolicy) + } + meta.SetStatusCondition(&newStatus.Conditions, *enforcedCondition) + propagateRecordConditions(recordsList, newStatus) + return newStatus } -func (r *DNSPolicyReconciler) enforcedCondition(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition { - recordsList := &kuadrantdnsv1alpha1.DNSRecordList{} - if err := r.Client().List(ctx, recordsList); err != nil { - r.Logger().V(1).Error(err, "error listing dns records") - return kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown(dnsPolicy.Kind(), err), false) - } - +func (r *DNSPolicyReconciler) enforcedCondition(recordsList *kuadrantdnsv1alpha1.DNSRecordList, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition { var controlled bool for _, record := range recordsList.Items { // check that DNS record is controller by this policy @@ -114,3 +121,39 @@ func (r *DNSPolicyReconciler) enforcedCondition(ctx context.Context, dnsPolicy * // there are no controlled DNS records present return kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown(dnsPolicy.Kind(), errors.New("policy is not enforced on any dns record: no routes attached for listeners")), false) } + +func propagateRecordConditions(records *kuadrantdnsv1alpha1.DNSRecordList, policyStatus *v1alpha1.DNSPolicyStatus) { + //reset conditions + policyStatus.ProbeConditions = map[string][]metav1.Condition{} + + for _, record := range records.Items { + var allConditions []metav1.Condition + allConditions = append(allConditions, record.Status.Conditions...) + if record.Status.HealthCheck != nil { + allConditions = append(allConditions, record.Status.HealthCheck.Conditions...) + + if record.Status.HealthCheck.Probes != nil { + for _, probeStatus := range record.Status.HealthCheck.Probes { + allConditions = append(allConditions, probeStatus.Conditions...) + } + } + } + + for _, condition := range allConditions { + //skip healthy negative polarity conditions + if slices.Contains(NegativePolarityConditions, condition.Type) && + strings.ToLower(string(condition.Status)) == "false" { + continue + } + //skip healthy positive polarity conditions + if !slices.Contains(NegativePolarityConditions, condition.Type) && + strings.ToLower(string(condition.Status)) == "true" { + continue + } + + policyStatus.ProbeConditions[*record.Spec.RootHost] = append( + policyStatus.ProbeConditions[*record.Spec.RootHost], + condition) + } + } +} diff --git a/controllers/dnspolicy_status_test.go b/controllers/dnspolicy_status_test.go new file mode 100644 index 000000000..2a166d087 --- /dev/null +++ b/controllers/dnspolicy_status_test.go @@ -0,0 +1,151 @@ +//go:build unit + +package controllers + +import ( + "reflect" + "testing" + + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" +) + +func TestPropagateRecordConditions(t *testing.T) { + healthyProviderCondition := metav1.Condition{ + Type: "Ready", + Status: "True", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + Reason: "ProviderSuccess", + Message: "Provider ensured the dns record", + } + + healthyProbesCondition := metav1.Condition{ + Type: "healthProbesSynced", + Status: "True", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + Reason: "AllProbesSynced", + Message: "all 1 probes synced successfully", + } + + healthyProbeCondition := metav1.Condition{ + Type: "ProbeSynced", + Status: "True", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + Reason: "ProbeSyncSuccessful", + Message: "probe (id: 687918a1-7127-4822-87fa-43afec1922cd, address: 172.32.0.253, host: test.domain.com) synced successfully", + } + + unhealthyProbesCondition := metav1.Condition{ + Type: "healthProbesSynced", + Status: "False", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + Reason: "UnsyncedProbes", + Message: "some probes have not yet successfully synced to the DNS Provider", + } + + unhealthyProbeCondition := metav1.Condition{ + Type: "ProbeSynced", + Status: "False", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + Reason: "DNSProviderError", + Message: "probe (id: , address: test.external.com, host: test.domain.com) error from DNS Provider: test.external.com is not a valid value of union type IPAddress.", + } + + rootHost := "test.domain.com" + + tests := []struct { + Name string + PolicyStatus *v1alpha1.DNSPolicyStatus + Records *kuadrantdnsv1alpha1.DNSRecordList + Validate func(*testing.T, *v1alpha1.DNSPolicyStatus) + }{ + { + Name: "Healthy conditions not propagated", + Records: &kuadrantdnsv1alpha1.DNSRecordList{ + Items: []kuadrantdnsv1alpha1.DNSRecord{ + { + Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: &rootHost}, + Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Conditions: []metav1.Condition{ + healthyProviderCondition, + }, + HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ + Conditions: []metav1.Condition{ + healthyProbesCondition, + }, + Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ + { + Conditions: []metav1.Condition{ + healthyProbeCondition, + }, + }, + }, + }, + }, + }, + }, + }, + PolicyStatus: &v1alpha1.DNSPolicyStatus{}, + Validate: func(t *testing.T, policyStatus *v1alpha1.DNSPolicyStatus) { + if conditions, ok := policyStatus.ProbeConditions[rootHost]; ok { + t.Fatalf("expected no probe conditions for root host, found %v", len(conditions)) + } + }, + }, + { + Name: "Unhealthy conditions are propagated", + Records: &kuadrantdnsv1alpha1.DNSRecordList{ + Items: []kuadrantdnsv1alpha1.DNSRecord{ + { + Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: &rootHost}, + Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Conditions: []metav1.Condition{ + healthyProviderCondition, + }, + HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ + Conditions: []metav1.Condition{ + unhealthyProbesCondition, + }, + Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ + { + Conditions: []metav1.Condition{ + unhealthyProbeCondition, + }, + }, + }, + }, + }, + }, + }, + }, + PolicyStatus: &v1alpha1.DNSPolicyStatus{}, + Validate: func(t *testing.T, policyStatus *v1alpha1.DNSPolicyStatus) { + if conditions, ok := policyStatus.ProbeConditions[rootHost]; !ok { + t.Fatalf("expected probe conditions for root host, found none") + } else { + if len(conditions) != 2 { + t.Fatalf("expected 2 probe conditions on policy, found %v", len(conditions)) + } + for _, c := range conditions { + if !reflect.DeepEqual(c, unhealthyProbeCondition) && !reflect.DeepEqual(c, unhealthyProbesCondition) { + t.Fatalf("unexpected condition: %v", c) + } + } + } + }, + }, + } + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + propagateRecordConditions(tt.Records, tt.PolicyStatus) + tt.Validate(t, tt.PolicyStatus) + }) + } +} diff --git a/go.mod b/go.mod index 85961e5e8..2c7ae1494 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/google/uuid v1.4.0 github.com/kuadrant/authorino v0.15.0 github.com/kuadrant/authorino-operator v0.9.0 - github.com/kuadrant/dns-operator v0.0.0-20240405095001-9e88ad4a7303 + github.com/kuadrant/dns-operator v0.0.0-20240426081919-e328d819392b // indirect github.com/kuadrant/limitador-operator v0.7.0 github.com/martinlindhe/base36 v1.1.1 github.com/onsi/ginkgo/v2 v2.13.2 diff --git a/go.sum b/go.sum index 4e4316aac..4aa9c8ce6 100644 --- a/go.sum +++ b/go.sum @@ -258,6 +258,10 @@ github.com/kuadrant/authorino-operator v0.9.0 h1:EV7zrYBNcd53HPQMivvTwe/+DIATTK7 github.com/kuadrant/authorino-operator v0.9.0/go.mod h1:VkUqS4CHNiaHMrjSFQ5V71DN829kPnqT3FQxqlOntEI= github.com/kuadrant/dns-operator v0.0.0-20240405095001-9e88ad4a7303 h1:3PFrKndF/xDHXbCCVX4W7Y6ir5oNw+hxuyBmwRb2hKM= github.com/kuadrant/dns-operator v0.0.0-20240405095001-9e88ad4a7303/go.mod h1:xT10T4H3uCXkB+xpW0KP9yb/k3zNAA2OXW9WR9j38kA= +github.com/kuadrant/dns-operator v0.0.0-20240426081919-e328d819392b h1:xQNP+EMa9/FniWCAfQEKNR1cPLANAEcWGNAcY1CIx0E= +github.com/kuadrant/dns-operator v0.0.0-20240426081919-e328d819392b/go.mod h1:5UhTSjazSNW/eW+pn1ZIeuvuPDnRMwiFkYgAlCoC9zI= +github.com/kuadrant/dns-operator v0.1.0 h1:MlSKdzNejuxDFhIjn6/OFcdYnLyMo95SRMlFs5WZZ+A= +github.com/kuadrant/dns-operator v0.1.0/go.mod h1:qmqqpvIRFewuTWd4kox/udz32hW7TQsE7Wvd45Eea18= github.com/kuadrant/limitador-operator v0.7.0 h1:pLIpM6vUxAY/Jn6ny61IGpqS7Oti786duBzJ67DJOuA= github.com/kuadrant/limitador-operator v0.7.0/go.mod h1:tg+G+3eTzUUfvUmdbiqH3FnScEPSWZ3DmorD1ZAx1bo= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=