Skip to content

Commit

Permalink
Use spec.Namespace to prevent collisions rather than ownerRef (knativ…
Browse files Browse the repository at this point in the history
…e#10503)

* Bump deps

* Use spec.Namespace to prevent collisions rather than ownerRef

* one import path to rule them all
  • Loading branch information
julz authored and nak3 committed Jan 21, 2021
1 parent 1547e63 commit 42ca1a5
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 14 deletions.
25 changes: 23 additions & 2 deletions pkg/reconciler/domainmapping/reconciler.go
Expand Up @@ -107,6 +107,27 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, dm *v1alpha1.DomainMappi
return err
}

// FinalizeKind cleans up the ClusterDomainClaim created by the DomainMapping.
func (r *Reconciler) FinalizeKind(ctx context.Context, dm *v1alpha1.DomainMapping) reconciler.Event {
dc, err := r.netclient.NetworkingV1alpha1().ClusterDomainClaims().Get(ctx, dm.Name, metav1.GetOptions{})
if err != nil {
if apierrs.IsNotFound(err) {
// Nothing to do since the domain was never claimed.
return nil
}

return err
}

// We need to check that we only delete if the CDC is owned by our namespace, otherwise we could
// delete the claim when we didn't succeed in acquiring it.
if dc.Spec.Namespace != dm.Namespace {
return nil
}

return r.netclient.NetworkingV1alpha1().ClusterDomainClaims().Delete(ctx, dm.Name, metav1.DeleteOptions{})
}

func (r *Reconciler) reconcileIngress(ctx context.Context, dm *v1alpha1.DomainMapping, desired *netv1alpha1.Ingress) (*netv1alpha1.Ingress, error) {
recorder := controller.GetEventRecorder(ctx)
ingress, err := r.ingressLister.Ingresses(desired.Namespace).Get(desired.Name)
Expand Down Expand Up @@ -187,9 +208,9 @@ func (r *Reconciler) reconcileDomainClaim(ctx context.Context, dm *v1alpha1.Doma
return fmt.Errorf("failed to get ClusterDomainClaim: %w", err)
}

if !metav1.IsControlledBy(dc, dm) {
if dm.Namespace != dc.Spec.Namespace {
dm.Status.MarkDomainClaimNotOwned()
return fmt.Errorf("domain mapping: %q does not own matching cluster domain claim", dm.Name)
return fmt.Errorf("domain mapping: namespace %q does not own cluster domain claim for %q", dm.Namespace, dm.Name)
}

dm.Status.MarkDomainClaimed()
Expand Down
11 changes: 6 additions & 5 deletions pkg/reconciler/domainmapping/resources/domainclaim.go
Expand Up @@ -19,17 +19,18 @@ package resources
import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
"knative.dev/pkg/kmeta"
"knative.dev/serving/pkg/apis/serving/v1alpha1"
)

// MakeDomainClaim creates a ClusterDomainClaim named after and owned by the
// given DomainMapping.
// MakeDomainClaim creates a ClusterDomainClaim named after the given DomainMapping
// and giving ownership of the domain name to the DomainMapping's namespace.
func MakeDomainClaim(dm *v1alpha1.DomainMapping) *netv1alpha1.ClusterDomainClaim {
return &netv1alpha1.ClusterDomainClaim{
ObjectMeta: metav1.ObjectMeta{
Name: dm.Name,
OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(dm)},
Name: dm.Name,
},
Spec: netv1alpha1.ClusterDomainClaimSpec{
Namespace: dm.Namespace,
},
}
}
15 changes: 10 additions & 5 deletions pkg/reconciler/domainmapping/resources/domainclaim_test.go
Expand Up @@ -22,18 +22,23 @@ import (
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
"knative.dev/pkg/kmeta"
"knative.dev/serving/pkg/apis/serving/v1alpha1"
)

func TestMakeDomainClaim(t *testing.T) {
dm := &v1alpha1.DomainMapping{ObjectMeta: metav1.ObjectMeta{Name: "mapping.com"}}
got := MakeDomainClaim(dm)
got := MakeDomainClaim(&v1alpha1.DomainMapping{
ObjectMeta: metav1.ObjectMeta{
Name: "mapping.com",
Namespace: "the-namespace",
},
})

want := &netv1alpha1.ClusterDomainClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "mapping.com",
OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(dm)},
Name: "mapping.com",
},
Spec: netv1alpha1.ClusterDomainClaimSpec{
Namespace: "the-namespace",
},
}

Expand Down

0 comments on commit 42ca1a5

Please sign in to comment.