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

fix: getHostByName #412

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lib0xidium
Copy link

  • added error handling to avoid panics

@lib0xidium
Copy link
Author

lib0xidium commented Oct 10, 2024

Hello @mattfarina,

Due to this line: https://github.com/Masterminds/sprig/blob/master/network.go#L11 Helm is failing when getHostByName returns nothing (e.g: due to NXDOMAIN).

The error looks like this:

error calling getHostByName: invalid argument to Intn

because rand.Intn() is called with 0 as an argument.

We should handle this case and return a more descriptive error, both in sprig and helm packages.

Also, returning a random entry from the resolved addresses might introduce confusion. I would prefer/propose to return the whole list of results.

- changed getHostByName
- added unit-test
@lib0xidium lib0xidium force-pushed the bugfix/gethostbyname-error-handling branch from e7c443f to 65d69fd Compare October 10, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant