Skip to content

Commit

Permalink
Don't allow gateway DNS records to be created if they clash with exis…
Browse files Browse the repository at this point in the history 
…ting ingress controllers.
  • Loading branch information
@rfredette
rfredette committed Oct 17, 2024
1 parent db0e811 commit 37d0f99
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 0 deletions.
25 changes: 25 additions & 0 deletions pkg/operator/controller/gateway-service-dns/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
corev1 "k8s.io/api/core/v1"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
iov1 "github.com/openshift/api/operatoringress/v1"

apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -241,8 +242,18 @@ func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *ga
Name: service.Name,
UID: service.UID,
}
ics := operatorv1.IngressControllerList{}
if err := r.cache.List(ctx, &ics); err != nil {
log.Error(err, "failed to list Ingress Controllers")
return []error{err}
}
var errs []error
for _, domain := range domains {
// Check if the domain matches one from an existing ingress controller. If so, log an error and ignore the invalid domain.
if err := domainClashes(domain, ics); err != nil {
log.Error(fmt.Errorf("error creating DNS record for gateway %s: %w", gateway.Name, err), "ignoring invalid gateway domain")
continue
}
name := operatorcontroller.GatewayDNSRecordName(gateway, domain)
dnsPolicy := iov1.UnmanagedDNS
if dnsrecord.ManageDNSForDomain(domain, infraConfig.Status.PlatformStatus, dnsConfig) {
Expand All @@ -254,6 +265,20 @@ func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *ga
return errs
}

// domainClashes checks if domain clashes with any of the wildcard records of the ingress controllers in icList. If
// there is a clash, an error is returned describing which ingress controller already uses the specified domain name.
func domainClashes(domain string, icList operatorv1.IngressControllerList) error {
for _, ic := range icList.Items {
if len(ic.Status.Domain) == 0 {
continue
}
if strings.Compare(domain, "*."+ic.Status.Domain+".") == 0 {
return fmt.Errorf("listener hostname %q conflicts with the domain from ingress controller %s.", domain, ic.Name)
}
}
return nil
}

// deleteStaleDNSRecordsForGateway deletes any DNSRecord CRs that are associated
// with the given gateway but specify a DNS name that is not in the given set of
// domains. Such DNSRecord CRs may exist if a hostname was modified or deleted
Expand Down
62 changes: 62 additions & 0 deletions pkg/operator/controller/gateway-service-dns/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
iov1 "github.com/openshift/api/operatoringress/v1"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -41,6 +42,17 @@ func Test_Reconcile(t *testing.T) {
},
},
}
ic := func(name, domain string) *operatorv1.IngressController {
return &operatorv1.IngressController{
ObjectMeta: metav1.ObjectMeta{
Namespace: "openshift-ingress-operator",
Name: name,
},
Status: operatorv1.IngressControllerStatus{
Domain: domain,
},
}
}
gw := func(name string, listeners ...gatewayapiv1beta1.Listener) *gatewayapiv1beta1.Gateway {
return &gatewayapiv1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -127,6 +139,7 @@ func Test_Reconcile(t *testing.T) {
infraConfig,
gw("example-gateway", l("stage-http", "*.stage.example.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
Expand All @@ -140,6 +153,7 @@ func Test_Reconcile(t *testing.T) {
dnsConfig,
gw("example-gateway", l("stage-http", "*.stage.example.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
Expand All @@ -153,6 +167,7 @@ func Test_Reconcile(t *testing.T) {
dnsConfig, infraConfig,
gw("example-gateway"),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
Expand All @@ -170,6 +185,7 @@ func Test_Reconcile(t *testing.T) {
l("prod-https", "*.prod.example.com", 443),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
Expand All @@ -190,6 +206,7 @@ func Test_Reconcile(t *testing.T) {
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("newlb.example.com")),
dnsrecord("example-gateway-7bdcfc8f68-wildcard", "*.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "oldlb.example.com"),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
Expand All @@ -208,6 +225,7 @@ func Test_Reconcile(t *testing.T) {
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
dnsrecord("example-gateway-64754456b8-wildcard", "*.old.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
Expand All @@ -224,6 +242,7 @@ func Test_Reconcile(t *testing.T) {
dnsConfig, infraConfig,
gw("example-gateway", l("stage-http", "*.stage.example.com", 80), l("stage-https", "*.stage.example.com", 443)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
Expand All @@ -238,6 +257,7 @@ func Test_Reconcile(t *testing.T) {
dnsConfig, infraConfig,
gw("example-gateway", l("http", "*.foo.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
Expand All @@ -246,12 +266,54 @@ func Test_Reconcile(t *testing.T) {
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
{
name: "gateway with two unique host names, one of which clashes with an existing ingress controller",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw(
"example-gateway",
l("stage-https", "*.stage.apps.example.com", 443),
l("apps-https", "*.apps.example.com", 443),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-644bf77744-wildcard", "*.stage.apps.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
{
name: "gateway with two unique host names, neither of which clashes with an existing ingress controller",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw(
"example-gateway",
l("stage-https", "*.stage.apps.example.com", 443),
// apps.example.com looks like it'll clash with the default ic below, but the ic creates a record
// for *.apps.example.com, which doesn't actually clash with apps.example.com
l("apps-https", "apps.example.com", 443),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
ic("default", "apps.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-644bf77744-wildcard", "*.stage.apps.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
dnsrecord("example-gateway-54b5446744-wildcard", "apps.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
}

scheme := runtime.NewScheme()
iov1.AddToScheme(scheme)
corev1.AddToScheme(scheme)
gatewayapiv1beta1.AddToScheme(scheme)
operatorv1.AddToScheme(scheme)

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down

0 comments on commit 37d0f99

Please sign in to comment.