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

Refactored chart-museum logic for E2E tests #5143

Merged
merged 4 commits into from
Aug 1, 2022
Merged

Conversation

castelblanque
Copy link
Collaborator

Signed-off-by: Rafa Castelblanque rcastelblanq@vmware.com

Description of the change

This PR tackles two issues:

  1. After Make ChartMuseum CI version configurable #5119 Charts museum version was upgraded. Parameters of the chart for user authentication have changed in the newer versions.

    Previous version installation command

    helm install chartmuseum --namespace chart-museum "https://github.com/chartmuseum/charts/releases/download/chartmuseum-3.8.0/chartmuseum-3.8.0.tgz" \
        --set env.open.DISABLE_API=false \
        --set persistence.enabled=true \
        --set secret.AUTH_USER=admin \
        --set secret.AUTH_PASS=password

    Authentication was not working with newer version + old parameters. ChartMuseum could be accessed without any authentication:

    kubectl run tmp-shell --restart=Never --rm -i --tty --image centos -- /bin/sh -c "curl http://chartmuseum.chart-museum.svc.cluster.local:8080/index.yaml"

    Newer versions installation command

    helm install chartmuseum --namespace chart-museum "https://github.com/chartmuseum/charts/releases/download/chartmuseum-3.8.0/chartmuseum-3.8.0.tgz" \
        --set env.open.DISABLE_API=false \
        --set persistence.enabled=true \
        --set env.secret.BASIC_AUTH_USER=admin \
        --set env.secret.BASIC_AUTH_PASS=password
  2. Scripting code used for managing ChartMuseum was mixed with the E2E code and scattered in a couple of files. This PR refactors that and keeps all ChartMuseum logic within a single file. It is also installed in its own namespace, instead of kubeapps. The script can be used externally as a regular script, I mean apart from E2E.

Benefits

Fix for authentication against ChartMuseum and tidier scripting code.

Possible drawbacks

N/A

Additional information

Rafa Castelblanque added 2 commits July 28, 2022 16:51
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
@castelblanque castelblanque marked this pull request as ready for review July 28, 2022 15:55
@netlify
Copy link

netlify bot commented Jul 28, 2022

Deploy Preview for kubeapps-dev canceled.

Name Link
🔨 Latest commit 7f2941e
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/62e78505b73c430008c13ec9

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has totally checked 1555 files.

Valid Invalid Ignored Fixed
758 1 796 0
Click to see the invalid file list
  • script/chart-museum.sh

script/chart-museum.sh Show resolved Hide resolved
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Great, nice improvements. It's good to have all the chartmuseum-related logic altogether.

Comment on lines 6 to 10
CM_PORT=${CM_PORT:-8090}
CM_USER=${CM_USER:-"admin"}
CM_PWD=${CM_PWD:-"password"}
CHART_MUSEUM_NS=${CHART_MUSEUM_NS:-"chart-museum"}
CM_VERSION=${CM_VERSION:-"3.9.0"}
Copy link
Contributor

Choose a reason for hiding this comment

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

As per #5119 (comment), I think we should use the same CHART_MUSEUM_ prefix, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, agree. Changed it to CHARTMUSEUM_ as in the e2e-test.sh file.

script/chart-museum.sh Show resolved Hide resolved
script/chart-museum.sh Show resolved Hide resolved
@castelblanque
Copy link
Collaborator Author

Thanks @antgamdia !

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
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.

None yet

3 participants