Skip to content

Commit

Permalink
NS Deletion hardening
Browse files Browse the repository at this point in the history
  • Loading branch information
@philbrookes
philbrookes committed Jun 10, 2024
1 parent 366a746 commit 58d2e7f
Show file tree
Hide file tree
Showing 7 changed files with 322 additions and 72 deletions.
10 changes: 10 additions & 0 deletions internal/common/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package common
import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
)

Expand All @@ -23,3 +24,12 @@ func RandomizeDuration(variance float64, duration time.Duration) time.Duration {
int64(lowerLimit),
int64(upperLimit)))
}

func Owns(owner, object metav1.Object) bool {
for _, ref := range object.GetOwnerReferences() {
if ref.UID == owner.GetUID() {
return true
}
}
return false
}
137 changes: 137 additions & 0 deletions internal/common/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ package common
import (
"testing"
"time"

. "github.com/onsi/gomega"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

"github.com/kuadrant/dns-operator/api/v1alpha1"
)

func TestRandomizeDuration(t *testing.T) {
Expand Down Expand Up @@ -41,3 +48,133 @@ func isValidVariance(duration, randomizedDuration time.Duration, variance float6
return float64(randomizedDuration.Milliseconds()) >= lowerLimmit &&
float64(randomizedDuration.Milliseconds()) < upperLimit
}

func TestOwns(t *testing.T) {
RegisterTestingT(t)
testCases := []struct {
Name string
Object metav1.Object
Owner metav1.Object
Verify func(t *testing.T, result bool)
}{
{
Name: "object is owned",
Object: &v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone",
UID: "unique-uid",
BlockOwnerDeletion: ptr.To(true),
},
},
},
},
Owner: &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
UID: "unique-uid",
},
},
Verify: func(t *testing.T, result bool) {
Expect(result).To(BeTrue())
},
}, {
Name: "object is owned by multiple",
Object: &v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone-other",
UID: "unique-uid-other",
BlockOwnerDeletion: ptr.To(true),
},
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone",
UID: "unique-uid",
BlockOwnerDeletion: ptr.To(true),
},
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone-other2",
UID: "unique-uid-other2",
BlockOwnerDeletion: ptr.To(true),
},
},
},
},
Owner: &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
UID: "unique-uid",
},
},
Verify: func(t *testing.T, result bool) {
Expect(result).To(BeTrue())
},
}, {
Name: "object is not owned",
Object: &v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone-other",
UID: "unique-uid-other",
BlockOwnerDeletion: ptr.To(false),
},
},
},
},
Owner: &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
UID: "unique-uid",
},
},
Verify: func(t *testing.T, result bool) {
Expect(result).To(BeFalse())
},
}, {
Name: "object is not owned multiple",
Object: &v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone-other",
UID: "unique-uid-other",
BlockOwnerDeletion: ptr.To(true),
}, {
APIVersion: "v1beta1",
Kind: "ManagedZone",
Name: "test-zone-other2",
UID: "unique-uid-other2",
BlockOwnerDeletion: ptr.To(false),
},
},
},
},
Owner: &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
UID: "unique-uid",
},
},
Verify: func(t *testing.T, result bool) {
Expect(result).To(BeFalse())
},
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
testCase.Verify(t, Owns(testCase.Owner, testCase.Object))
})
}
}
112 changes: 48 additions & 64 deletions internal/controller/dnsrecord_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"time"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -30,7 +31,6 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
externaldnsendpoint "sigs.k8s.io/external-dns/endpoint"
Expand Down Expand Up @@ -96,11 +96,36 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
}
dnsRecord := previous.DeepCopy()

managedZone := &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
Name: dnsRecord.Spec.ManagedZoneRef.Name,
Namespace: dnsRecord.Namespace,
},
}
err = r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{})
if err != nil {
reason := "ProviderError"
message := fmt.Sprintf("The DNS provider failed to ensure the record: %v", provider.SanitizeError(err))
setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message)
return r.updateStatus(ctx, previous, dnsRecord, false, err)
}

// Ensure provider secret exists before continuing
err = r.Get(ctx, client.ObjectKey{Namespace: managedZone.Namespace, Name: managedZone.Spec.SecretRef.Name}, &v1.Secret{}, &client.GetOptions{})
if err != nil {
secretMissing.WithLabelValues(managedZone.Name, managedZone.Namespace, managedZone.Spec.SecretRef.Name).Set(1)
reason := "ProviderError"
message := fmt.Sprintf("The DNS provider failed to ensure the record: %v", provider.SanitizeError(err))
setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message)
return r.updateStatus(ctx, previous, dnsRecord, false, err)
}
secretMissing.WithLabelValues(managedZone.Name, managedZone.Namespace, managedZone.Spec.SecretRef.Name).Set(0)

if dnsRecord.DeletionTimestamp != nil && !dnsRecord.DeletionTimestamp.IsZero() {
if err = r.ReconcileHealthChecks(ctx, dnsRecord); client.IgnoreNotFound(err) != nil {
if err = r.ReconcileHealthChecks(ctx, dnsRecord, managedZone); client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, err
}
hadChanges, err := r.deleteRecord(ctx, dnsRecord)
hadChanges, err := r.deleteRecord(ctx, dnsRecord, managedZone)
if err != nil {
logger.Error(err, "Failed to delete DNSRecord")
return ctrl.Result{}, err
Expand Down Expand Up @@ -134,6 +159,14 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{RequeueAfter: randomizedValidationRequeue}, nil
}

if !common.Owns(managedZone, dnsRecord) {
err = controllerutil.SetOwnerReference(managedZone, dnsRecord, r.Scheme)
if err != nil {
return ctrl.Result{}, err
}
err = r.Client.Update(ctx, dnsRecord)
return ctrl.Result{}, err
}
var reason, message string
err = dnsRecord.Validate()
if err != nil {
Expand All @@ -144,15 +177,15 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
}

// Publish the record
hadChanges, err := r.publishRecord(ctx, dnsRecord)
hadChanges, err := r.publishRecord(ctx, managedZone, dnsRecord)
if err != nil {
reason = "ProviderError"
message = fmt.Sprintf("The DNS provider failed to ensure the record: %v", provider.SanitizeError(err))
setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message)
return r.updateStatus(ctx, previous, dnsRecord, hadChanges, err)
}

if err = r.ReconcileHealthChecks(ctx, dnsRecord); err != nil {
if err = r.ReconcileHealthChecks(ctx, dnsRecord, managedZone); err != nil {
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -237,42 +270,14 @@ func (r *DNSRecordReconciler) SetupWithManager(mgr ctrl.Manager, maxRequeue, val

return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.DNSRecord{}).
Watches(&v1alpha1.ManagedZone{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {
logger := log.FromContext(ctx)
toReconcile := []reconcile.Request{}
// list dns records in the maanagedzone namespace as they will be in the same namespace as the zone
records := &v1alpha1.DNSRecordList{}
if err := mgr.GetClient().List(ctx, records, &client.ListOptions{Namespace: o.GetNamespace()}); err != nil {
logger.Error(err, "failed to list dnsrecords ", "namespace", o.GetNamespace())
return toReconcile
}
for _, record := range records.Items {
if record.Spec.ManagedZoneRef.Name == o.GetName() {
logger.Info("managed zone updated", "managedzone", o.GetNamespace()+"/"+o.GetName(), "enqueuing dnsrecord ", record.GetName())
toReconcile = append(toReconcile, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&record)})
}
}
return toReconcile
})).
Complete(r)
}

// deleteRecord deletes record(s) in the DNSPRovider(i.e. route53) configured by the ManagedZone assigned to this
// DNSRecord (dnsRecord.Status.ParentManagedZone).
func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (bool, error) {
func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone) (bool, error) {
logger := log.FromContext(ctx)

managedZone := &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
Name: dnsRecord.Spec.ManagedZoneRef.Name,
Namespace: dnsRecord.Namespace,
},
}
err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{})
if err != nil {
// If the Managed Zone isn't found, just continue
return false, client.IgnoreNotFound(err)
}
managedZoneReady := meta.IsStatusConditionTrue(managedZone.Status.Conditions, "Ready")

if !managedZoneReady {
Expand All @@ -297,18 +302,9 @@ func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alp

// publishRecord publishes record(s) to the DNSPRovider(i.e. route53) configured by the ManagedZone assigned to this
// DNSRecord (dnsRecord.Status.ParentManagedZone).
func (r *DNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (bool, error) {
func (r *DNSRecordReconciler) publishRecord(ctx context.Context, managedZone *v1alpha1.ManagedZone, dnsRecord *v1alpha1.DNSRecord) (bool, error) {
logger := log.FromContext(ctx)
managedZone := &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
Name: dnsRecord.Spec.ManagedZoneRef.Name,
Namespace: dnsRecord.Namespace,
},
}
err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{})
if err != nil {
return false, err
}

managedZoneReady := meta.IsStatusConditionTrue(managedZone.Status.Conditions, "Ready")

if !managedZoneReady {
Expand Down Expand Up @@ -355,14 +351,14 @@ func exponentialRequeueTime(lastRequeueTime string) time.Duration {
return randomizedValidationRequeue
}
// double the duration. Return the max timeout if overshoot
newReqeueue := lastRequeue * 2
if newReqeueue > defaultRequeueTime {
newRequeue := lastRequeue * 2
if newRequeue > defaultRequeueTime {
return defaultRequeueTime
}
return newReqeueue
return newRequeue
}

// setDNSRecordCondition adds or updates a given condition in the DNSRecord status..
// setDNSRecordCondition adds or updates a given condition in the DNSRecord status.
func setDNSRecordCondition(dnsRecord *v1alpha1.DNSRecord, conditionType string, status metav1.ConditionStatus, reason, message string) {
cond := metav1.Condition{
Type: conditionType,
Expand All @@ -374,18 +370,7 @@ func setDNSRecordCondition(dnsRecord *v1alpha1.DNSRecord, conditionType string,
meta.SetStatusCondition(&dnsRecord.Status.Conditions, cond)
}

func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (provider.Provider, error) {
managedZone := &v1alpha1.ManagedZone{
ObjectMeta: metav1.ObjectMeta{
Name: dnsRecord.Spec.ManagedZoneRef.Name,
Namespace: dnsRecord.Namespace,
},
}
err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{})
if err != nil {
return nil, err
}

func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, managedZone *v1alpha1.ManagedZone) (provider.Provider, error) {
providerConfig := provider.Config{
DomainFilter: externaldnsendpoint.NewDomainFilter([]string{managedZone.Spec.DomainName}),
ZoneTypeFilter: externaldnsprovider.NewZoneTypeFilter(""),
Expand All @@ -403,9 +388,8 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
rootDomainName, _ := strings.CutPrefix(dnsRecord.Spec.RootHost, v1alpha1.WildcardPrefix)
zoneDomainFilter := externaldnsendpoint.NewDomainFilter([]string{zoneDomainName})
managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME}
excludeDNSRecordTypes := []string{}

dnsProvider, err := r.getDNSProvider(ctx, dnsRecord)
var excludeDNSRecordTypes []string
dnsProvider, err := r.getDNSProvider(ctx, managedZone)
if err != nil {
return false, err
}
Expand Down
Loading

0 comments on commit 58d2e7f

Please sign in to comment.