Skip to content

Commit

Permalink
Merge pull request #934 from Miciah/OCPBUGS-10875-gateway-service-dns…
Browse files Browse the repository at this point in the history 
…-set-DNS-policy-appropriately

OCPBUGS-10875: gateway-service-dns: Set DNS policy appropriately
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Aug 5, 2023
2 parents 833bc28 + 620c930 commit 2be8f80
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 60 deletions.
26 changes: 23 additions & 3 deletions pkg/operator/controller/gateway-service-dns/controller.go
View file
Expand Up @@ -2,6 +2,7 @@ package gateway_service_dns

import (
"context"
"fmt"
"reflect"
"strings"

Expand All @@ -15,6 +16,7 @@ import (

corev1 "k8s.io/api/core/v1"

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

apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -184,9 +186,23 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
return reconcile.Result{}, err
}

dnsConfig := &configv1.DNS{}
if err := r.client.Get(ctx, types.NamespacedName{Name: "cluster"}, dnsConfig); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to get dns 'cluster': %v", err)
}

infraConfig := &configv1.Infrastructure{}
if err := r.client.Get(ctx, types.NamespacedName{Name: "cluster"}, infraConfig); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to get infrastructure 'cluster': %v", err)
}
if infraConfig.Status.PlatformStatus == nil {
log.Info("infrastructure 'cluster' has nil status.platformStatus; reconciliation will be skipped")
return reconcile.Result{}, nil
}

domains := getGatewayHostnames(&gateway)
var errs []error
errs = append(errs, r.ensureDNSRecordsForGateway(ctx, &gateway, &service, domains.List())...)
errs = append(errs, r.ensureDNSRecordsForGateway(ctx, &gateway, &service, domains.List(), infraConfig, dnsConfig)...)
errs = append(errs, r.deleteStaleDNSRecordsForGateway(ctx, &gateway, &service, domains)...)
return reconcile.Result{}, utilerrors.NewAggregate(errs)
}
Expand All @@ -212,7 +228,7 @@ func getGatewayHostnames(gateway *gatewayapiv1beta1.Gateway) sets.String {
// ensureDNSRecordsForGateway ensures that a DNSRecord CR exists, associated
// with the given gateway and service, for each of the given domains. It
// returns a list of any errors that result from ensuring those DNSRecord CRs.
func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *gatewayapiv1beta1.Gateway, service *corev1.Service, domains []string) []error {
func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *gatewayapiv1beta1.Gateway, service *corev1.Service, domains []string, infraConfig *configv1.Infrastructure, dnsConfig *configv1.DNS) []error {
labels := map[string]string{
gatewayNameLabelKey: gateway.Name,
}
Expand All @@ -228,7 +244,11 @@ func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *ga
var errs []error
for _, domain := range domains {
name := operatorcontroller.GatewayDNSRecordName(gateway, domain)
_, _, err := dnsrecord.EnsureDNSRecord(r.client, name, labels, ownerRef, domain, service)
dnsPolicy := iov1.UnmanagedDNS
if dnsrecord.ManageDNSForDomain(domain, infraConfig.Status.PlatformStatus, dnsConfig) {
dnsPolicy = iov1.ManagedDNS
}
_, _, err := dnsrecord.EnsureDNSRecord(r.client, name, labels, ownerRef, domain, dnsPolicy, service)
errs = append(errs, err)
}
return errs
Expand Down
175 changes: 121 additions & 54 deletions pkg/operator/controller/gateway-service-dns/controller_test.go
View file
Expand Up @@ -10,6 +10,7 @@ import (

gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

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

corev1 "k8s.io/api/core/v1"
Expand All @@ -26,6 +27,20 @@ import (
)

func Test_Reconcile(t *testing.T) {
dnsConfig := &configv1.DNS{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Spec: configv1.DNSSpec{
BaseDomain: "example.com",
},
}
infraConfig := &configv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
},
},
}
gw := func(name string, listeners ...gatewayapiv1beta1.Listener) *gatewayapiv1beta1.Gateway {
return &gatewayapiv1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -62,14 +77,21 @@ func Test_Reconcile(t *testing.T) {
},
}
}
gatewayManagedLabel := map[string]string{
"gateway.istio.io/managed": "example-gateway",
}
exampleGatewayLabel := map[string]string{
"istio.io/gateway-name": "example-gateway",
}
ingHost := func(hostname string) corev1.LoadBalancerIngress {
return corev1.LoadBalancerIngress{
Hostname: hostname,
}
}
dnsrecord := func(name, dnsName string, targets ...string) *iov1.DNSRecord {
dnsrecord := func(name, dnsName string, policy iov1.DNSManagementPolicy, labels map[string]string, targets ...string) *iov1.DNSRecord {
return &iov1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
Namespace: "openshift-ingress",
Name: name,
},
Expand All @@ -78,7 +100,7 @@ func Test_Reconcile(t *testing.T) {
RecordType: iov1.CNAMERecordType,
Targets: targets,
RecordTTL: 30,
DNSManagementPolicy: iov1.ManagedDNS,
DNSManagementPolicy: policy,
},
}
}
Expand All @@ -96,103 +118,133 @@ func Test_Reconcile(t *testing.T) {
reconcileRequest reconcile.Request
expectCreate []client.Object
expectUpdate []client.Object
expectDelete []client.Object
expectError string
}{
{
name: "missing dns config",
existingObjects: []runtime.Object{
infraConfig,
gw("example-gateway", l("stage-http", "*.stage.example.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
expectError: `dnses.config.openshift.io "cluster" not found`,
},
{
name: "missing infrastructure config",
existingObjects: []runtime.Object{
dnsConfig,
gw("example-gateway", l("stage-http", "*.stage.example.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
expectError: `infrastructures.config.openshift.io "cluster" not found`,
},
{
name: "gateway with no listeners",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw("example-gateway"),
svc(
"example-gateway",
map[string]string{
"gateway.istio.io/managed": "example-gateway",
},
map[string]string{
"istio.io/gateway-name": "example-gateway",
},
ingHost("lb.example.com"),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
{
name: "gateway with three listeners and two unique host names, no dnsrecords",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw(
"example-gateway",
l("stage-http", "*.stage.example.com", 80),
l("stage-https", "*.stage.example.com", 443),
l("prod-https", "*.prod.example.com", 443),
),
svc(
"example-gateway",
map[string]string{
"gateway.istio.io/managed": "example-gateway",
},
map[string]string{
"istio.io/gateway-name": "example-gateway",
},
ingHost("lb.example.com"),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-76456f8647-wildcard", "*.prod.example.com.", "lb.example.com"),
dnsrecord("example-gateway-64754456b8-wildcard", "*.stage.example.com.", "lb.example.com"),
dnsrecord("example-gateway-76456f8647-wildcard", "*.prod.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
dnsrecord("example-gateway-64754456b8-wildcard", "*.stage.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
{
name: "gateway with two listeners and one dnsrecord with a stale target, hostname already has trailing dot",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw(
"example-gateway",
l("http", "*.example.com", 80),
l("https", "*.example.com", 443),
),
svc(
"example-gateway",
map[string]string{
"gateway.istio.io/managed": "example-gateway",
},
map[string]string{
"istio.io/gateway-name": "example-gateway",
},
ingHost("newlb.example.com"),
),
dnsrecord("example-gateway-7bdcfc8f68-wildcard", "*.example.com.", "oldlb.example.com"),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("newlb.example.com")),
dnsrecord("example-gateway-7bdcfc8f68-wildcard", "*.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "oldlb.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{},
expectUpdate: []client.Object{
dnsrecord("example-gateway-7bdcfc8f68-wildcard", "*.example.com.", "newlb.example.com"),
dnsrecord("example-gateway-7bdcfc8f68-wildcard", "*.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "newlb.example.com"),
},
expectDelete: []client.Object{},
},
{
name: "gateway with two listeners and one host name, no dnsrecords, name ends up with trailing dot",
name: "gateway with a stale dnsrecord",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw(
"example-gateway",
l("stage-http", "*.stage.example.com", 80),
l("stage-https", "*.stage.example.com", 443),
),
svc(
"example-gateway",
map[string]string{
"gateway.istio.io/managed": "example-gateway",
},
map[string]string{
"istio.io/gateway-name": "example-gateway",
},
ingHost("lb.example.com"),
l("http", "*.new.example.com", 80),
),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
dnsrecord("example-gateway-64754456b8-wildcard", "*.old.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-68ffc6d64-wildcard", "*.new.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{
dnsrecord("example-gateway-64754456b8-wildcard", "*.old.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
},
{
name: "gateway with two listeners and one host name, no dnsrecords, name ends up with trailing dot",
existingObjects: []runtime.Object{
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")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-64754456b8-wildcard", "*.stage.example.com.", "lb.example.com"),
dnsrecord("example-gateway-64754456b8-wildcard", "*.stage.example.com.", iov1.ManagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
{
name: "gateway with a listener with an unmanaged domain, no dnsrecords",
existingObjects: []runtime.Object{
dnsConfig, infraConfig,
gw("example-gateway", l("http", "*.foo.com", 80)),
svc("example-gateway", gatewayManagedLabel, exampleGatewayLabel, ingHost("lb.example.com")),
},
reconcileRequest: req("openshift-ingress", "example-gateway"),
expectCreate: []client.Object{
dnsrecord("example-gateway-795d4b47fd-wildcard", "*.foo.com.", iov1.UnmanagedDNS, exampleGatewayLabel, "lb.example.com"),
},
expectUpdate: []client.Object{},
expectDelete: []client.Object{},
},
}

Expand All @@ -207,7 +259,7 @@ func Test_Reconcile(t *testing.T) {
WithScheme(scheme).
WithRuntimeObjects(tc.existingObjects...).
Build()
cl := &fakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}}
cl := &fakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}, []client.Object{}}
informer := informertest.FakeInformers{Scheme: scheme}
cache := fakeCache{Informers: &informer, Reader: cl}
reconciler := &reconciler{
Expand All @@ -218,8 +270,14 @@ func Test_Reconcile(t *testing.T) {
client: cl,
}
res, err := reconciler.Reconcile(context.Background(), tc.reconcileRequest)
if assert.NoError(t, err) {
assert.Equal(t, reconcile.Result{}, res)
if tc.expectError == "" {
if assert.NoError(t, err) {
assert.Equal(t, reconcile.Result{}, res)
}
} else {
if assert.Error(t, err) {
assert.Contains(t, err.Error(), tc.expectError)
}
}
cmpOpts := []cmp.Option{
cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Finalizers", "Labels", "OwnerReferences", "ResourceVersion"),
Expand All @@ -231,6 +289,11 @@ func Test_Reconcile(t *testing.T) {
if diff := cmp.Diff(tc.expectUpdate, cl.updated, cmpOpts...); diff != "" {
t.Fatalf("found diff between expected and actual updates: %s", diff)
}
// A deleted object has zero spec.
delCmpOpts := append(cmpOpts, cmpopts.IgnoreTypes(iov1.DNSRecordSpec{}))
if diff := cmp.Diff(tc.expectDelete, cl.deleted, delCmpOpts...); diff != "" {
t.Fatalf("found diff between expected and actual deletes: %s", diff)
}
})
}
}
Expand All @@ -246,6 +309,7 @@ type fakeClientRecorder struct {

added []client.Object
updated []client.Object
deleted []client.Object
}

func (c *fakeClientRecorder) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
Expand All @@ -266,11 +330,13 @@ func (c *fakeClientRecorder) RESTMapper() meta.RESTMapper {

func (c *fakeClientRecorder) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
c.added = append(c.added, obj)
c.T.Log(obj)
c.T.Log("CREATE", obj)
return c.Client.Create(ctx, obj, opts...)
}

func (c *fakeClientRecorder) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
c.deleted = append(c.deleted, obj)
c.T.Log("DELETE", obj)
return c.Client.Delete(ctx, obj, opts...)
}

Expand All @@ -280,6 +346,7 @@ func (c *fakeClientRecorder) DeleteAllOf(ctx context.Context, obj client.Object,

func (c *fakeClientRecorder) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
c.updated = append(c.updated, obj)
c.T.Log("UPDATE", obj)
return c.Client.Update(ctx, obj, opts...)
}

Expand Down

0 comments on commit 2be8f80

Please sign in to comment.