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.

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

allow coredns override extensions #7583

Merged
merged 1 commit into from May 31, 2023
Merged

allow coredns override extensions #7583

merged 1 commit into from May 31, 2023

Conversation

ghost
Copy link

@ghost ghost commented May 19, 2023

Proposed Changes

Allow CoreDNS override extensions in default Corefile

Other K8s implementations such as AKS[1] and DigitalOcean[2] have similar configuration

This would enable ConfigMaps to adjust the CoreDNS settings without creating a whole copy of coredns.yaml.

1: https://learn.microsoft.com/en-us/azure/aks/coredns-custom
2: https://docs.digitalocean.com/products/kubernetes/how-to/customize-coredns/

Types of Changes

New Feature

Verification

  1. Patch Corefile with import /etc/coredns/custom/*.override in the .:53 default server block
  2. Create a coredns-custom ConfigMap with some custom settings for the default server block, e.g. rewrite name substring svc.example.com svc.cluster.local
  3. Restart CoreDNS pod
  4. Test that the custom config has taken effect. For my example, run nslookup my-pod.my-svc.my-namespace.svc.example.com or getent hosts ... in a pod and confirm it resolves to the same name as my-pod.my-svc.my-namespace.svc.cluster.local

Testing

Linked Issues

User-Facing Change


Further Comments

@ghost ghost self-requested a review as a code owner May 19, 2023 01:28
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! You need to make download generate and check in the resulting changes.

@cwayne18
Copy link
Member

Also as a CNCF project we need to have commits signed, can you please make sure to commit with -s so the DCO check passes? Thanks a lot for the PR!

@ghost
Copy link
Author

ghost commented May 20, 2023

Thanks for checking @cwayne18 @brandond. I've just updated the commit with my signature and checked in the re-generated Go artifact.

@brandond
Copy link
Member

brandond commented May 22, 2023

@andrewroffey your commits are signed (-S/--gpg-sign), but not signed off (-s/--signoff). DCO requires signoff, not signature.

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.17 ⚠️

Comparison is base (d069a85) 47.71% compared to head (71b5001) 47.55%.

❗ Current head 71b5001 differs from pull request most recent head 6872eb6. Consider uploading reports for the commit 6872eb6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7583      +/-   ##
==========================================
- Coverage   47.71%   47.55%   -0.17%     
==========================================
  Files         140      140              
  Lines       14284    14284              
==========================================
- Hits         6816     6793      -23     
- Misses       6399     6413      +14     
- Partials     1069     1078       +9     
Flag Coverage Δ
inttests 44.91% <ø> (-0.17%) ⬇️
unittests 19.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/deploy/zz_generated_bindata.go 51.49% <ø> (ø)

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ghost
Copy link
Author

ghost commented May 23, 2023

@andrewroffey your commits are signed (-S/--gpg-sign), but not signed off (-s/--signoff). DCO requires signoff, not signature.

@brandond oops, third time lucky, TIL about the -s flag. I've updated that with both this time.

Copy link
Member

@dereknola dereknola left a comment

Choose a reason for hiding this comment

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

We are still in code freeze for May releases, but this will get in after that, should be in all June releases.

@cwayne18
Copy link
Member

Thanks a lot for the PR, and for signing your commits :) Like Derek said we'll get this in as soon as code freeze is thawed

@ghost
Copy link
Author

ghost commented May 25, 2023

Cheers! That's just around the corner, sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants