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

Increase debug of helm tests #1153

Merged
merged 5 commits into from Sep 6, 2019
Merged

Conversation

andresmgot
Copy link
Contributor

Ref #1146

Sometimes the helm tests fails but since we are not printing the logs we are not sure what could be happening. The goal of this PR is to add verbosity to the tests so we can find the cause of the issue when it appears.

@@ -18,5 +18,5 @@ spec:
command:
- sh
- -c
- "curl -ik -H \"Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)\" $TILLER_PROXY_HOST:$TILLER_PROXY_PORT/v1/releases | grep $KUBEAPPS_RELEASE"
- "curl -v -o /tmp/output -ik -H \"Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)\" $TILLER_PROXY_HOST:$TILLER_PROXY_PORT/v1/releases && cat /tmp/output && cat /tmp/output | grep $KUBEAPPS_RELEASE"
Copy link
Member

Choose a reason for hiding this comment

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

I guess these clusters are temporary, but here we may expose sensitive information to the CI logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to say no because this is meant for the local cluster using kind but the same is executed for GKE and showing the headers would show a valid token for the cluster. Good catch! I am removing the -v

if [[ "$code" != 0 ]]; then
echo "PODS status on failure"
kubectl get pods -n kubeapps
for pod in $(kubectl get po -l release=kubeapps-ci -oname -n kubeapps); do
Copy link
Member

Choose a reason for hiding this comment

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

Same here about exposing sensitive data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in this case there is nothing sensitive for the temporary cluster

Copy link
Member

@Angelmmiguel Angelmmiguel left a comment

Choose a reason for hiding this comment

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

LGTM! Please, check my comments before merging the changes

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Similar to Angel's question, wondering where this output is going to be publicly visible? CircleCI? What credentials could potentially be exposed, if any?

@@ -16,5 +16,5 @@ spec:
command:
- sh
- -c
- curl $CHARTSVC_HOST:$CHARTSVC_PORT/v1/charts | grep wordpress
- curl -o /tmp/output $CHARTSVC_HOST:$CHARTSVC_PORT/v1/charts && cat /tmp/output && cat /tmp/output | grep wordpress
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you cating the whole /tmp/output then also redoing with the pipe? (here and everywhere).
That just means we see some of the output twice, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we just do the grep pipe and the output doesn't contain what we expect it to contain (like for example an error), the actual content would be filtered out. That's why I am showing the output and then do the "grep" as the test.

@andresmgot
Copy link
Contributor Author

Similar to Angel's question, wondering where this output is going to be publicly visible? CircleCI? What credentials could potentially be exposed, if any?

Yes, in CircleCI. We are showing the logs of the pods of the temporary cluster. We are not showing any credentials there. You can see an example of what's being printed here:

(note that I have removed the -v from the curl calls in the tests so the headers won't be shown)

https://circleci.com/gh/kubeapps/kubeapps/9043?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@andresmgot andresmgot merged commit 959d608 into vmware-tanzu:master Sep 6, 2019
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.

None yet

3 participants