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

queryhosts: enforce Array[String] data type #101

Merged
merged 3 commits into from
Nov 17, 2020

Conversation

kenyon
Copy link
Member

@kenyon kenyon commented Nov 16, 2020

The conversion from ERB to EPP in #79 changed the behavior of queryhosts slightly. Before, setting queryhosts to the empty string worked for configuring chrony to allow any query. After, empty string is considered empty, so the "allow" directive would not be rendered.

Enforcing the data type ensures that people upgrading this module who have been using empty string get an error rather than a valid but misconfigured chrony.conf. The replacement for empty string is an array with a single empty string element: ['']

Also, fixing a typo in the tests.

I also removed an obsolete vscode extensions.json (the puppet extension name is obsolete). I have the whole .vscode directory git-ignored, so bundle exec rake test aborts when it sees that something ignored exists in the repository. I don't think we really need this file.

@kenyon
Copy link
Member Author

kenyon commented Nov 16, 2020

The test failures are unrelated to this change.

@alexjfisher
Copy link
Member

I've reopened #93 but need to resolve conflicts.

The conversion from ERB to EPP in voxpupuli#79 changed the behavior of
queryhosts slightly. Before, setting queryhosts to the empty string
worked for configuring chrony to allow any query. After, empty string
is considered empty, so the "allow" directive would not be rendered.

Enforcing the data type ensures that people upgrading this module who
have been using empty string get an error rather than a valid but
misconfigured chrony.conf. The replacement for empty string is an
array with a single empty string element: ['']
This file shouldn't be in the repository. Its existence in the
repository causes rake test to fail if you have this gitignored. This
isn't the current name of the puppet vscode extension, and it's not
necessary to have this file anyway.
@kenyon kenyon force-pushed the allow-empty-queryhosts branch from f0fea09 to ea71bb6 Compare November 16, 2020 22:09
@bastelfreak bastelfreak added the bug Something isn't working label Nov 17, 2020
@kenyon kenyon merged commit cc390d7 into voxpupuli:master Nov 17, 2020
kenyon added a commit to kenyon/puppet-chrony that referenced this pull request Dec 10, 2022
Commit ee2d9de changed the data type
for queryhosts from Array[String] to Array[String[1]]. These
parameters configure `allow` and `deny` in chrony.conf. But `allow` and
`deny` are allowed to be by themselves in the config, meaning "allow
everything" or "deny everything": https://chrony.tuxfamily.org/doc/4.3/chrony.conf.html#allow

This commit corrects the data type to Array[String[0]] and adds a test
to prevent future breakage (this was broken before too, and I fixed it
again in voxpupuli#101).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants