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

Make Coredns config configurable #74

Merged
merged 10 commits into from
Dec 16, 2023

Conversation

rwaffen
Copy link
Member

@rwaffen rwaffen commented Dec 14, 2023

  • CoreDNS config map is exported to a template with a variable set for the path so that one can bring its own config map if desired.
  • the template uses full qualified variable names, so that when the template path is changed one also can add more variables without touching the manifest and updating the variables hash of the template

@rwaffen
Copy link
Member Author

rwaffen commented Dec 14, 2023

i updated the PR to use hiera, i hope this is like you wanted it?

@ananace
Copy link
Member

ananace commented Dec 14, 2023

I'd personally prefer a hiera override, but with the main template still rendered in Puppet if there's no value provided from there - since that works better for ENC use-cases.

@rwaffen
Copy link
Member Author

rwaffen commented Dec 15, 2023

but when i set the hiera value in the module data, there will always be a value for this. wont it? or do i not understand this in the right way?

but you say i shall build it this way that we check for hiera, if empty we render the template?

- so it can be lookedup for sure from the coredns hiera data
@ananace
Copy link
Member

ananace commented Dec 15, 2023

What I meant with the ENC use-case is when you have an external node classifier directly providing the values - instead of setting them through Hiera, i.e. something that amounts to;

class { 'k8s':
  cluster_domain => 'example.com',
}

Hiera lookups won't find values set directly through puppet or an ENC;

$ cat example.pp 
class something (
  String[1] $value
) {
}

class { 'something':
  value => 'potatoes',
}

notice(lookup('something::value'))
$ puppet apply example.pp
Error: Function lookup() did not find a value for the name 'something::value'

@rwaffen
Copy link
Member Author

rwaffen commented Dec 15, 2023

ah okay, i don't use ENC very often and try to avoid it if i can. but now i understand you.

- use hiera if defined, else use template
@rwaffen rwaffen requested a review from ananace December 15, 2023 09:34
@rwaffen rwaffen added the enhancement New feature or request label Dec 15, 2023
@rwaffen rwaffen requested a review from ananace December 15, 2023 16:25
Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

I still personally feel that a content parameter should be enough of a public API for configuring CoreDNS, template rendering is mostly class-internal complexity after all. But that's mainly just personal preference, and nothing that should prevent this from being merged if it's useful to people.

@rwaffen rwaffen merged commit ac8e752 into voxpupuli:master Dec 16, 2023
3 checks passed
@rwaffen rwaffen deleted the make_coredns_config_configurable branch December 16, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants