Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CoreDNS rewrite support for external services. #656

Closed
wants to merge 21 commits into from
Closed

Add CoreDNS rewrite support for external services. #656

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5211ffd
CoreDNS rewrite support.
aviweit Jun 18, 2024
083a46c
Update policy in instance_controller.go.
aviweit Jun 18, 2024
fdcd150
Update configmap rbac.
aviweit Jun 18, 2024
b287952
Update pkg/controlplane/control/manager.go
aviweit Jun 19, 2024
52ad6af
Update pkg/apis/clusterlink.net/v1alpha1/import.go
aviweit Jun 19, 2024
bb9b316
Rename DNSName to Alias.
aviweit Jun 19, 2024
e7dad17
Updates per make codegen.
aviweit Jun 19, 2024
6e337cf
Updates per dnsName to alias rename and apply nit suggestions.
aviweit Jun 19, 2024
92c266a
Move coredns related functions to dns.go.
aviweit Jun 19, 2024
8d7cda9
Update pkg/controlplane/control/dns.go
aviweit Jun 20, 2024
d6b94fa
Update pkg/controlplane/control/manager.go
aviweit Jun 20, 2024
b31c0a3
Pass client, logger to dns methods.
aviweit Jun 20, 2024
bafabdc
Merge branch 'main' into corednsrewrite
aviweit Jun 20, 2024
cbe72e2
Patch coredns deployment to trigger pod restart.
aviweit Jun 24, 2024
1e251a5
Merge branch 'main' into corednsrewrite
aviweit Jun 24, 2024
f73b4b9
Lint.
aviweit Jun 25, 2024
805194c
Lint.
aviweit Jun 25, 2024
40d67ce
Lint.
aviweit Jun 25, 2024
f0cb126
Add configmaps to role at bootstrap/platform/k8s.go.
aviweit Jun 25, 2024
caf3536
Lint.
aviweit Jun 25, 2024
4177a82
Lint and make precommit.
aviweit Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/crds/clusterlink.net_imports.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ spec:
spec:
description: Spec represents the attributes of the imported service.
properties:
dnsName:
description: DnsName is an optional external dns name for this imported
aviweit marked this conversation as resolved.
Show resolved Hide resolved
service
type: string
lbScheme:
default: round-robin
description: LBScheme is the load-balancing scheme to use (e.g., random,
Expand Down
10 changes: 10 additions & 0 deletions config/operator/rbac/role.yaml
View file
Open in desktop
aviweit marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ kind: ClusterRole
metadata:
name: cl-operator-manager-role
rules:
- apiGroups:
aviweit marked this conversation as resolved.
Show resolved Hide resolved
- ""
resources:
- configmaps
verbs:
- get
- list
- update
- watch
- apiGroups:
- ""
resources:
Expand All @@ -17,6 +26,7 @@ rules:
resources:
- pods
verbs:
- delete
- get
- list
- watch
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/clusterlink.net/v1alpha1/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ type ImportSpec struct {
// +kubebuilder:default="round-robin"
// LBScheme is the load-balancing scheme to use (e.g., random, static, round-robin)
LBScheme LBScheme `json:"lbScheme"`
// DNSName is an optional external DNS name for this imported service
DNSName string `json:"dnsName,omitempty"`
}

const (
Expand Down
187 changes: 173 additions & 14 deletions pkg/controlplane/control/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,98 @@ type Manager struct {
logger *logrus.Entry
}

// Restart coredns pods
func coreDnsRestart(ctx context.Context, m *Manager) error {
aviweit marked this conversation as resolved.
Show resolved Hide resolved
aviweit marked this conversation as resolved.
Show resolved Hide resolved
var pods v1.PodList
if err := m.client.List(ctx, &pods, client.InNamespace("kube-system")); err != nil {
return err
}

for _, pod := range pods.Items {
if strings.Contains(pod.Name, "coredns") {
m.logger.Infof("Deleting pod: %s/%s.", pod.Namespace, pod.Name)
err := m.client.Delete(ctx, &pod)
if err != nil {
return err
}
}
}

return nil
}

// Add coredns rewrite for a given external dns service
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be less intrusive to use the hosts or file plugins so that the changes are not embedded as part of the CoreDNS configuration but are in their own file.
We could then ask an administrator to enable the CoreDNS plugin manually (if they so desire) and require lower privileges for us (we won't be mocking with the Corefile directly).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elevran , looking into hosts plugin, in case it is configured with malformed syntax, coredns pods seem to also fail.

$ kubectl --kubeconfig ~/.kube/cl-peer2  get pod   -n kube-system
NAME                                             READY   STATUS             RESTARTS      AGE
coredns-5d78c9869d-klk2t                         0/1     CrashLoopBackOff   5 (61s ago)   4m11s

$ kubectl --kubeconfig ~/.kube/cl-peer2  logs coredns-5d78c9869d-klk2t    -n kube-system
/etc/coredns/custom.hosts:1 - Error during parsing: Unknown directive '1.2.3.4'

coredns configmap:

apiVersion: v1
data:
  Corefile: |
    .:53 {
        import custom.hosts                    < ---- added
        errors
        health {
           lameduck 5s
        }
        ready
        kubernetes cluster.local in-addr.arpa ip6.arpa {
           pods insecure
           fallthrough in-addr.arpa ip6.arpa
           ttl 30
        }
        prometheus :9153
        forward . /etc/resolv.conf {
           max_concurrent 1000
        }
        cache 30
        loop
        reload
        loadbalance
        hosts custom.hosts                    < ---- added
    }
  custom.hosts: |                    < ---- added
    1.2.3.4 mycustom.host
kind: ConfigMap

coredns deployment:

...
      containers:
      - args:
        - -conf
        - /etc/coredns/Corefile
        image: registry.k8s.io/coredns/coredns:v1.10.1
        imagePullPolicy: IfNotPresent
...
        volumeMounts:
        - mountPath: /etc/coredns
          name: config-volume
          readOnly: true

...
      volumes:
      - configMap:
          defaultMode: 420
          items:
          - key: Corefile
            path: Corefile
          - key: custom.hosts                    < ---- added
            path: custom.hosts                    < ---- added
          name: coredns
        name: config-volume

func addCoreDnsRewrite(ctx context.Context, m *Manager, name *types.NamespacedName, dnsName string) error {
corednsName := types.NamespacedName{
Name: "coredns",
Namespace: "kube-system",
}
var cm v1.ConfigMap

if err := m.client.Get(ctx, corednsName, &cm); err != nil {
if k8serrors.IsNotFound(err) {
m.logger.Warnf("coredns configmap not found.")
return nil
} else {
return err
}
}
if data, ok := cm.Data["Corefile"]; ok {
// remove trailing end-of-line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you use the file / hosts plugins there might already be packages for parsing their data (I imagine the pluiin needs it anyway)

dataEol := strings.TrimSuffix(data, "\n")
aviweit marked this conversation as resolved.
Show resolved Hide resolved
// break into lines
lines := strings.Split(dataEol, "\n")
serviceFqdn := fmt.Sprintf("%s.%s.svc.cluster.local", name.Name, name.Namespace)

var coreFileUpdated = false
var rewriteLine = ""
for i, line := range lines {
if strings.Contains(line, serviceFqdn) {
// matched line already exists
break
}
// ready marker is reached - matched line not found, append it here
if strings.Contains(line, " ready") {
if strings.HasPrefix(dnsName, "*.") { // wildcard DNS
dnsName = strings.TrimPrefix(dnsName, "*")
dnsName = strings.ReplaceAll(dnsName, ".", "\\.")
dnsName = "(.*)" + dnsName

rewriteLine = fmt.Sprintf(" rewrite name regex %s %s answer auto", dnsName, serviceFqdn)
aviweit marked this conversation as resolved.
Show resolved Hide resolved
} else {
rewriteLine = fmt.Sprintf(" rewrite name %s %s", dnsName, serviceFqdn)
}
// add matched line
lines = append(lines[:i+1], lines[i:]...)
lines[i] = rewriteLine
coreFileUpdated = true
break
}
}

if coreFileUpdated {
// update configmap and restart the pods
var newLines string = ""
for _, line := range lines {
// return back EOL
newLines += (line + "\n")
}
cm.Data["Corefile"] = newLines
if err := m.client.Update(ctx, &cm); err != nil {
return err
}

if err := coreDnsRestart(ctx, m); err != nil {
return err
}
}
} else {
return errors.New("coredns configmap['Corefile'] not found")
}

return nil
}

// AddImport adds a listening socket for an imported remote service.
func (m *Manager) AddImport(ctx context.Context, imp *v1alpha1.Import) (err error) {
m.logger.Infof("Adding import '%s/%s'.", imp.Namespace, imp.Name)
Expand Down Expand Up @@ -243,31 +335,96 @@ func (m *Manager) AddImport(ctx context.Context, imp *v1alpha1.Import) (err erro
return err
}

if imp.Namespace == m.namespace {
return nil
if imp.Namespace != m.namespace {
userService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: imp.Name,
Namespace: imp.Namespace,
Labels: make(map[string]string),
},
Spec: v1.ServiceSpec{
ExternalName: fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, m.namespace),
Type: v1.ServiceTypeExternalName,
},
}

if err := m.addImportService(ctx, imp, userService); err != nil {
return err
}
}

userService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: imp.Name,
Namespace: imp.Namespace,
Labels: make(map[string]string),
},
Spec: v1.ServiceSpec{
ExternalName: fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, m.namespace),
Type: v1.ServiceTypeExternalName,
},
if imp.Spec.DnsName != "" {
if err := addCoreDnsRewrite(ctx, m, &importName, imp.Spec.DnsName); err != nil {
m.logger.Errorf("Failed to configure CoreDns: %v.", err)
aviweit marked this conversation as resolved.
Show resolved Hide resolved
return err
}
}
return nil

}

// Remove coredns rewrite for a given external dns service
func removeCoreDnsRewrite(ctx context.Context, m *Manager, name *types.NamespacedName) error {
corednsName := types.NamespacedName{
Name: "coredns",
Namespace: "kube-system",
}
var cm v1.ConfigMap

return m.addImportService(ctx, imp, userService)
if err := m.client.Get(ctx, corednsName, &cm); err != nil {
if k8serrors.IsNotFound(err) {
m.logger.Warnf("coredns configmap not found.")
return nil
} else {
return err
}
}
if data, ok := cm.Data["Corefile"]; ok {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may want to move rewrite parsing into its own package/struct so that the file parsing is not split across these functions as internal implementation

// remove trailing end-of-line
dataEol := strings.TrimSuffix(data, "\n")
// break into lines
lines := strings.Split(dataEol, "\n")
serviceFqdn := fmt.Sprintf("%s.%s.svc.cluster.local", name.Name, name.Namespace)

var coreFileUpdated = false
for i, line := range lines {
if strings.Contains(line, serviceFqdn) {
// remove matched line
lines = append(lines[:i], lines[i+1:]...)
coreFileUpdated = true
break
}
}

if coreFileUpdated {
// update configmap and restart the pods
var newLines string = ""
for _, line := range lines {
// return back EOL
newLines += (line + "\n")
}
cm.Data["Corefile"] = newLines
if err := m.client.Update(ctx, &cm); err != nil {
return err
}

if err := coreDnsRestart(ctx, m); err != nil {
return err
}
}
} else {
return errors.New("coredns configmap['Corefile'] not found")
}

return nil
}

// DeleteImport removes the listening socket of a previously imported service.
func (m *Manager) DeleteImport(ctx context.Context, name types.NamespacedName) error {
m.logger.Infof("Deleting import '%s/%s'.", name.Namespace, name.Name)

// delete user service
errs := make([]error, 3)
errs := make([]error, 4)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I realize it is not part of the new code, but I wonder if using append won't be a cleaner solution instead of assigning specific array entries. Unless, of course, there is a reliance on the specific order even when there are nil errors in the array.
Alternately, this might be better served by errors.Join().
@kfirtoledo @orozery - not familiar with this code well enough to say. Please weigh in.

errs[0] = m.deleteImportService(ctx, name, name)

if name.Namespace != m.namespace {
Expand All @@ -284,6 +441,8 @@ func (m *Manager) DeleteImport(ctx context.Context, name types.NamespacedName) e

m.ports.Release(name)

errs[3] = removeCoreDnsRewrite(ctx, m, &name)

return errors.Join(errs...)
}

Expand Down
12 changes: 10 additions & 2 deletions pkg/operator/controller/instance_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ type InstanceReconciler struct {
// +kubebuilder:rbac:groups="",resources=services;serviceaccounts,verbs=list;get;watch;create;update;patch;delete
// +kubebuilder:rbac:groups="discovery.k8s.io",resources=endpointslices,verbs=list;get;watch;create;update;patch;delete
// +kubebuilder:rbac:groups="",resources=nodes,verbs=list;get;watch
// +kubebuilder:rbac:groups="",resources=pods,verbs=list;get;watch
// +kubebuilder:rbac:groups="",resources=pods,verbs=list;get;delete;watch
aviweit marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;update;watch
// +kubebuilder:rbac:groups=clusterlink.net,resources=exports;peers;accesspolicies;privilegedaccesspolicies,verbs=list;get;watch
aviweit marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:rbac:groups=clusterlink.net,resources=imports,verbs=get;list;watch;update
// +kubebuilder:rbac:groups=clusterlink.net,resources=peers/status;exports/status;imports/status,verbs=update
Expand Down Expand Up @@ -428,6 +429,13 @@ func (r *InstanceReconciler) createAccessControl(ctx context.Context, name, name
"get", "list", "watch", "create", "delete", "update",
},
},
{
APIGroups: []string{""},
Resources: []string{"configmaps"},
Verbs: []string{
"get", "list", "update", "watch",
},
},
aviweit marked this conversation as resolved.
Show resolved Hide resolved
{
APIGroups: []string{"discovery.k8s.io"},
Resources: []string{"endpointslices"},
Expand All @@ -438,7 +446,7 @@ func (r *InstanceReconciler) createAccessControl(ctx context.Context, name, name
{
APIGroups: []string{""},
Resources: []string{"pods"},
Verbs: []string{"get", "list", "watch"},
Verbs: []string{"get", "delete", "list", "watch"},
aviweit marked this conversation as resolved.
Show resolved Hide resolved
},
{
APIGroups: []string{"clusterlink.net"},
Expand Down