Skip to content

Conversation

@portante
Copy link
Member

@portante portante commented Apr 8, 2021

From issue #2201, it became apparent that we weren't validating hostnames, which led to weird, but valid, directory names for
controllers being created on the server. One way this occurs is when the _pbench_hostname and or _pbench_full_hostname environment variables are defined with invalid values.

We add the new commands validate-hostname and validate-ipaddress to ensure the definition of those names are correct.

In the process of doing that, we found that we were using invalid host names in two of our unit tests.


Derived from PR #2181.

@portante portante force-pushed the validate-hostname branch from 6a07a82 to 3de19c6 Compare April 8, 2021 20:12
@portante portante requested a review from ndokos April 8, 2021 20:26
@portante portante marked this pull request as ready for review April 8, 2021 20:29
ndokos
ndokos previously approved these changes Apr 8, 2021
Copy link
Member Author

@portante portante left a comment

Choose a reason for hiding this comment

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

This should be good to go. I have addressed all the review comments from PR #2181 that were for this code.

Copy link
Member Author

@portante portante left a comment

Choose a reason for hiding this comment

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

Okay, now it is ready for re-review.

@portante portante requested a review from ndokos April 8, 2021 21:34
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

It looks like you got most of the items that I raised in #2181; I've transferred the remaining two.

@portante portante force-pushed the validate-hostname branch from 82212e6 to c941abb Compare April 8, 2021 23:11
@portante portante requested a review from webbnh April 8, 2021 23:12
From issue distributed-system-analysis#2201, it became apparent that we weren't validating
hostnames, which led to weird, but valid, directory names for
controllers being created on the server.  One way this occurs is when
the `_pbench_hostname` and or `_pbench_full_hostname` environment
variables are defined with invalid values.

We add the new commands `validate-hostname` and `validate-ipaddress`
to ensure the definition of those names are correct.

In the process of doing that, we found that we were using invalid host
names in two of our unit tests.
@portante portante force-pushed the validate-hostname branch from c941abb to 6c01204 Compare April 9, 2021 11:41
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks good, but before you merge we need to decide how to dispose of the issue below (see my comment).

@portante portante merged commit 2d79619 into distributed-system-analysis:main Apr 10, 2021
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.

4 participants