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

Require Unbound 1.6.6 or newer #287

Merged
merged 2 commits into from Jul 20, 2022
Merged

Conversation

b4ldr
Copy link
Member

@b4ldr b4ldr commented Oct 26, 2021

Pull Request (PR) description

This PR updates the minimum supported version of unbund. this allows
us to be a bit more generous when writing the first unbound.conf file
and fixes #286

I have picked unbound 1.6.6 as the minimum supported version. this
seems to be the version available in centos 7 which i assume is the
oldest version we support based on metatdata.json

This Pull Request (PR) fixes the following issues

Fixes: #286

@bastelfreak bastelfreak changed the title Update Minimum unbound version Require Unbound 1.6.6 or newer Jun 6, 2022
@bastelfreak
Copy link
Member

@b4ldr can you take a look at the failing tests?

@b4ldr
Copy link
Member Author

b4ldr commented Jun 6, 2022

@b4ldr can you take a look at the failing tests?

done, at least im pretty sure the unbound issues are unrelated but will double check

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

This feels like a major version bump to me. Perhaps we should first merge the other open PRs, get a release out and then do this major? Either way, I'm fine.

@b4ldr
Copy link
Member Author

b4ldr commented Jun 8, 2022

This feels like a major version bump to me. Perhaps we should first merge the other open PRs, get a release out and then do this major?

SGTM

@vox-pupuli-tasks
Copy link

Dear @b4ldr, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM, my only concern is that nothing says '1.6.6' in the code :-). I am not familiar with this code base, and I let you judge if it makes sense to have the print_config function raise an error when passed a version older than the oldest supported version so that the CI can help locating settings that need an update when we change this version.

@b4ldr
Copy link
Member Author

b4ldr commented Jun 28, 2022

This feels like a major version bump to me. Perhaps we should first merge the other open PRs, get a release out and then do this major? Either way, I'm fine.

done

LGTM, my only concern is that nothing says '1.6.6' in the code :-). I am not familiar with this code base, and I let you judge if it makes sense to have the print_config function raise an error when passed a version older than the oldest supported version so that the CI can help locating settings that need an update when we change this version.

personally i think its a bit overkill to add a minimum version in the print_config function. however we should state somewhere that the minimum version of unbound supported is 1.6.6. perhaps something in the metadata.json requirements block and.or something in the README.md?

@ekohl
Copy link
Member

ekohl commented Jun 28, 2022

README is IMHO the best place.

metadata.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@b4ldr
Copy link
Member Author

b4ldr commented Jul 14, 2022

updated

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

Looks fine, just the dubious requirement…

metadata.json Outdated Show resolved Hide resolved
This PR updates the minimum supported version of unbund.  this allows
us to be a bit more generous when writing the first unbound.conf file
and fixes voxpupuli#286

I have picked unbound 1.6.6 as the minimum supported version.  this
seems to be the version available in centos 7 which i assume is the
oldest version we support based on metatdata.json
README.md Outdated Show resolved Hide resolved
Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
@ekohl ekohl merged commit 3b166ee into voxpupuli:master Jul 20, 2022
@b4ldr b4ldr deleted the minimum_version branch July 20, 2022 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unbound_version not set on first run causing unexpected config file setting
4 participants