diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 26db6149f..bb0638caa 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -68,6 +68,11 @@ type DNSPolicySpec struct { // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:MinItems=1 ProviderRefs []dnsv1alpha1.ProviderRef `json:"providerRefs"` + + // ExcludeAddresses is a list of addresses (either hostnames, CIDR or IPAddresses) that DNSPolicy should not use as values in the configured DNS provider records. The default is to allow all addresses configured in the Gateway DNSPolicy is targeting + // +optional + // +kubebuilder:validation:MaxItems=20 + ExcludeAddresses []string `json:"excludeAddresses,omitempty"` } type LoadBalancingSpec struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 51b71361a..53a868b6c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -146,6 +146,11 @@ func (in *DNSPolicySpec) DeepCopyInto(out *DNSPolicySpec) { *out = make([]apiv1alpha1.ProviderRef, len(*in)) copy(*out, *in) } + if in.ExcludeAddresses != nil { + in, out := &in.ExcludeAddresses, &out.ExcludeAddresses + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSPolicySpec. diff --git a/config/crd/bases/kuadrant.io_dnspolicies.yaml b/config/crd/bases/kuadrant.io_dnspolicies.yaml index 32aaf6407..4c282be0b 100644 --- a/config/crd/bases/kuadrant.io_dnspolicies.yaml +++ b/config/crd/bases/kuadrant.io_dnspolicies.yaml @@ -65,6 +65,15 @@ spec: spec: description: DNSPolicySpec defines the desired state of DNSPolicy properties: + excludeAddresses: + description: ExcludeAddresses is a list of addresses (either hostnames, + CIDR or IPAddresses) that DNSPolicy should not use as values in + the configured DNS provider records. The default is to allow all + addresses configured in the Gateway DNSPolicy is targeting + items: + type: string + maxItems: 20 + type: array healthCheck: description: |- HealthCheckSpec configures health checks in the DNS provider. diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index 4147276d1..749c6efc6 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -133,7 +133,7 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy } if err = r.reconcileDNSRecords(ctx, dnsPolicy, gatewayDiffObj); err != nil { - return fmt.Errorf("reconcile DNSRecords error %w", err) + return fmt.Errorf("error reconciling DNSRecords %w", err) } // set direct back ref - i.e. claim the target network object as taken asap diff --git a/controllers/dnspolicy_dnsrecords.go b/controllers/dnspolicy_dnsrecords.go index 4b5193c76..75a6f7e12 100644 --- a/controllers/dnspolicy_dnsrecords.go +++ b/controllers/dnspolicy_dnsrecords.go @@ -35,7 +35,7 @@ func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) { log.V(1).Info("reconcileDNSRecords: gateway with valid or missing policy ref", "key", gw.Key()) if err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil { - return fmt.Errorf("error reconciling dns records for gateway %v: %w", gw.Gateway.Name, err) + return fmt.Errorf("reconciling dns records for gateway %v: error %w", gw.Gateway.Name, err) } } return nil @@ -52,6 +52,12 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw return err } + //ensure only approved addresses are considered for DNS records + if err := gatewayWrapper.SetValidStatusAddresses(dnsPolicy); err != nil { + log.V(3).Info("error setting valid addresses based on DNSPolicy") + return err + } + if err := r.dnsHelper.removeDNSForDeletedListeners(ctx, gatewayWrapper.Gateway); err != nil { log.V(3).Info("error removing DNS for deleted listeners") return err @@ -98,6 +104,14 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw return err } + if len(dnsRecord.Spec.Endpoints) == 0 { + log.V(1).Info("no endpoint addresses for DNSRecord ", "removing any records for listener", listener) + if err := r.dnsHelper.deleteDNSRecordForListener(ctx, gatewayWrapper, listener); client.IgnoreNotFound(err) != nil { + return err + } + return fmt.Errorf("no valid addresses for DNSRecord endpoints. Check allowedAddresses") + } + err = r.ReconcileResource(ctx, &kuadrantdnsv1alpha1.DNSRecord{}, dnsRecord, dnsRecordBasicMutator) if err != nil && !apierrors.IsAlreadyExists(err) { log.Error(err, "ReconcileResource failed to create/update DNSRecord resource") diff --git a/examples/dnspolicy/dnspolicy-exclude-address.yaml b/examples/dnspolicy/dnspolicy-exclude-address.yaml new file mode 100644 index 000000000..64be973a7 --- /dev/null +++ b/examples/dnspolicy/dnspolicy-exclude-address.yaml @@ -0,0 +1,20 @@ +apiVersion: kuadrant.io/v1alpha1 +kind: DNSPolicy +metadata: + name: prod-web + namespace: ${DNSPOLICY_NAMESPACE} +spec: + targetRef: + name: prod-web-istio + group: gateway.networking.k8s.io + kind: Gateway + providerRefs: + - name: aws-credentials + loadBalancing: + weight: 120 + geo: EU + defaultGeo: true + excludeAddresses: + - "10.89.0.0/16" + - "some.local.domain" + - "127.0.0.1" diff --git a/pkg/multicluster/gateway_wrapper.go b/pkg/multicluster/gateway_wrapper.go index 91326611f..d901be906 100644 --- a/pkg/multicluster/gateway_wrapper.go +++ b/pkg/multicluster/gateway_wrapper.go @@ -4,6 +4,9 @@ import ( "fmt" "strings" + "net" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -24,6 +27,37 @@ func NewGatewayWrapper(g *gatewayapiv1.Gateway, clusterID string) *GatewayWrappe return &GatewayWrapper{Gateway: g, ClusterID: clusterID} } +func (g *GatewayWrapper) SetValidStatusAddresses(p *v1alpha1.DNSPolicy) error { + newAddresses := []gatewayapiv1.GatewayStatusAddress{} + for _, address := range g.Gateway.Status.Addresses { + found := false + for _, exclude := range p.Spec.ExcludeAddresses { + if strings.LastIndex(exclude, "/") == len(exclude)-3 { + _, network, err := net.ParseCIDR(exclude) + if err != nil { + return fmt.Errorf("could not parse the CIDR from the excludeAddresses field %s", err) + } + ip := net.ParseIP(address.Value) + // only check addresses that are actually IPs + if ip != nil && network.Contains(ip) { + found = true + break + } + } + if exclude == address.Value { + found = true + break + } + } + if !found { + newAddresses = append(newAddresses, address) + } + } + // setting this in memory only wont be saved to actual gateway + g.Status.Addresses = newAddresses + return nil +} + func isMultiClusterAddressType(addressType gatewayapiv1.AddressType) bool { return addressType == MultiClusterIPAddressType || addressType == MultiClusterHostnameAddressType } diff --git a/pkg/multicluster/gateway_wrapper_test.go b/pkg/multicluster/gateway_wrapper_test.go new file mode 100644 index 000000000..abe965d74 --- /dev/null +++ b/pkg/multicluster/gateway_wrapper_test.go @@ -0,0 +1,131 @@ +package multicluster_test + +import ( + "testing" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/multicluster" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func TestSetValidStatusAddresses(t *testing.T) { + ipaddress := gatewayapiv1.IPAddressType + hostaddress := gatewayapiv1.HostnameAddressType + testCases := []struct { + Name string + Gateway *gatewayapiv1.Gateway + DNSPolicy *v1alpha1.DNSPolicy + Validate func(t *testing.T, g *gatewayapiv1.GatewayStatus) + }{ + { + Name: "ensure addresses in ingore are are removed from status", + Gateway: &gatewayapiv1.Gateway{ + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: &ipaddress, + Value: "1.1.1.1", + }, + { + Type: &hostaddress, + Value: "example.com", + }, + }, + }, + }, + DNSPolicy: &v1alpha1.DNSPolicy{ + Spec: v1alpha1.DNSPolicySpec{ + ExcludeAddresses: []string{ + "1.1.1.1", + }, + }, + }, + Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) { + if len(g.Addresses) != 1 { + t.Fatalf("expected a single address but got %v ", len(g.Addresses)) + } + for _, addr := range g.Addresses { + if addr.Value == "1.1.1.1" { + t.Fatalf("did not expect address %s to be present", "1.1.1.1") + } + } + }, + }, + { + Name: "ensure all addresses if nothing ignored", + Gateway: &gatewayapiv1.Gateway{ + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: &ipaddress, + Value: "1.1.1.1", + }, + { + Type: &hostaddress, + Value: "example.com", + }, + }, + }, + }, + DNSPolicy: &v1alpha1.DNSPolicy{ + Spec: v1alpha1.DNSPolicySpec{ + ExcludeAddresses: []string{}, + }, + }, + Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) { + if len(g.Addresses) != 2 { + t.Fatalf("expected a both address but got %v ", len(g.Addresses)) + } + }, + }, + { + Name: "ensure addresses removed if CIDR is set and hostname", + Gateway: &gatewayapiv1.Gateway{ + Status: gatewayapiv1.GatewayStatus{ + Addresses: []gatewayapiv1.GatewayStatusAddress{ + { + Type: &ipaddress, + Value: "1.1.1.1", + }, + { + Type: &hostaddress, + Value: "example.com", + }, + { + Type: &ipaddress, + Value: "81.17.21.22", + }, + }, + }, + }, + DNSPolicy: &v1alpha1.DNSPolicy{ + Spec: v1alpha1.DNSPolicySpec{ + ExcludeAddresses: []string{ + "1.1.0.0/16", + "10.0.0.1/32", + "example.com", + }, + }, + }, + Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) { + if len(g.Addresses) != 1 { + t.Fatalf("expected only a single address but got %v %v ", len(g.Addresses), g.Addresses) + } + if g.Addresses[0].Value != "81.17.21.22" { + t.Fatalf("expected the only remaining address to be 81.17.21.22 but got %s", g.Addresses[0].Value) + } + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + gw := multicluster.NewGatewayWrapper(tc.Gateway, "id") + err := gw.SetValidStatusAddresses(tc.DNSPolicy) + if err != nil { + t.Fatalf("unexpected error %s", err) + } + tc.Validate(t, &gw.Status) + }) + } +}