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

Add tests for InitNetClient. Refactor to pass chart details #1124

Merged

Conversation

absoludity
Copy link
Contributor

This is the next small installment for #1110 .

I'm planning to have InitNetClient fetching the AppRepository custom resource, when present in the request, but wasn't confident to change it before adding tests. So this PR just tests the existing functionality, with a tiny refactor to pass the ChartDetails into the call (as this will include the auth object for the current Details.Auth.CustomCA secret ref, as well as the Details.AppRepository.ResourceName which I'll pull from k8s and parse in a later PR).

I have one other small prep branch next, before I'll add the fetch of the custom resource in InitNetClient.

@absoludity absoludity self-assigned this Aug 19, 2019
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the tests!

@@ -311,7 +313,11 @@ func (c *Chart) InitNetClient(customCA *CustomCA) (HTTPClient, error) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

not related to your changes but re-reading this I think we should return the error (as you added) instead of logging a fatal error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woah, most definitely. I'll do that and the one I actually meant (up on 301 above), in the next PR.

@absoludity absoludity merged commit 746ee38 into vmware-tanzu:master Aug 19, 2019
@absoludity absoludity deleted the tiller-fetch-repo-resource branch August 19, 2019 09:48
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.

2 participants