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

aii-dhcp: support optional hostname ip verification #299

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stdweird
Copy link
Member

@stdweird stdweird commented Sep 13, 2018

Includes #296, based on #280

@stdweird stdweird added this to the 18.9 milestone Sep 13, 2018
@jrha jrha modified the milestones: 18.9, 18.12 Oct 10, 2018
@jrha jrha modified the milestones: 19.12, 20.2 Dec 2, 2019
@@ -20,4 +20,6 @@ type structure_dhcp_dhcp_info = {
the value you specify in templates.
}
"options" ? string{}
@{Verify hostname in DNS}
"verifyhostname" ? boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be choice("yes", "no", "strict") instead of a boolean, where strict would cause the component to fail if gethostbyname() below fails. Usually, not having a hard dependency on DNS being up-to-date is good, but I can think of a few use cases where it would be useful if AII could send the message: You need to wait for DNS to get updated, because otherwise the build will fail later anyway.

Copy link
Member

Choose a reason for hiding this comment

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

@stdweird any thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

wow, this is long time ago.
sure, but perhaps then choice("yes", "no", "warning") or something. or check_only instead of warning.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah too long ago really, sorry 😞

My vote is for "warn".

@jrha jrha modified the milestones: 20.3, 20.4 Mar 19, 2020
@jrha jrha removed this from the 22.12 milestone Jan 3, 2023
gombasg and others added 2 commits September 13, 2024 11:06
Add tests, adapted from aii-core. Replace bare open() with
CAF::FileReader to make the tests work. Include some improvements which
accumulated on our side.
@jrha jrha added this to the 25.next milestone Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants