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 Milestone Job integration tests for Helm Chart #171

Merged
merged 6 commits into from
Aug 20, 2020

Conversation

sigalsax
Copy link
Contributor

@sigalsax sigalsax commented Aug 2, 2020

This PR adds integration tests for the Milestone Job for the Secrets Provider

** NOTE that tests will not on Jenkins because Helm chart was not pushed.

You can run tests manually by:

  1. You need Helm chart locally to run helm chart successfully. you can check out locally via git checkout origin/helm-chart-phase-2 -- secrets-provider

  2. Run ./bin/start --oss --gke

@sigalsax sigalsax self-assigned this Aug 2, 2020
@sigalsax sigalsax marked this pull request as draft August 2, 2020 13:15
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 2 times, most recently from 8419795 to da7ffc1 Compare August 2, 2020 13:19
Comment on lines 21 to 31
sed -e "s#{{ SECRETS_PROVIDER_ROLE }}#${SECRETS_PROVIDER_ROLE:-"secrets-provider-role"}#g" \
-e "s#{{ SECRETS_PROVIDER_ROLE_BINDING }}#${SECRETS_PROVIDER_ROLE_BINDING:-"secrets-provider-role-binding"}#g" \
-e "s#{{ SERVICE_ACCOUNT_CREATE }}#${SERVICE_ACCOUNT_CREATE:-"true"}#g" \
-e "s#{{ SERVICE_ACCOUNT }}#${SERVICE_ACCOUNT:-"secrets-provider-service-account"}#g" \
-e "s#{{ K8S_SECRETS }}#${K8S_SECRETS:-"test-k8s-secret"}#g" \
-e "s#{{ CONJUR_ACCOUNT }}#${CONJUR_ACCOUNT:-"cucumber"}#g" \
-e "s#{{ CONJUR_APPLIANCE_URL }}#${CONJUR_APPLIANCE_URL:-"https://conjur-follower.${CONJUR_NAMESPACE_NAME}.svc.cluster.local/api"}#g" \
-e "s#{{ CONJUR_AUTHN_URL }}#${CONJUR_AUTHN_URL:-"https://conjur-follower.${CONJUR_NAMESPACE_NAME}.svc.cluster.local/api/authn-k8s/${AUTHENTICATOR_ID}"}#g" \
-e "s#{{ CONJUR_AUTHN_LOGIN }}# ${CONJUR_AUTHN_LOGIN:-"host/conjur/authn-k8s/${AUTHENTICATOR_ID}/apps/${APP_NAMESPACE_NAME}/*/*"}#g" \
-e "s#{{ SECRETS_PROVIDER_SSL_CONFIG_MAP }}# ${SECRETS_PROVIDER_SSL_CONFIG_MAP:-"secrets-provider-ssl-config-map"}#g" \
"secrets-provider-chart/ci/test-values-template.yaml" > "secrets-provider-chart/ci/test-values.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks messy.
You can make use of envsubst, which is a GNU tool, instead of this.
See for more info: https://stackoverflow.com/a/14157575

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 agree but since we are moving to BDD in the near future and this PR has been open for a while this seems less pressing.
This is part of a greater refactoring effort that could be addressed during our RD Boost days

@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 3 times, most recently from 0f7dcb1 to e530601 Compare August 3, 2020 13:27
bootstrap.env Outdated Show resolved Hide resolved
deploy/run_with_summon.sh Outdated Show resolved Hide resolved
Tovli
Tovli previously requested changes Aug 4, 2020
Copy link
Contributor

@Tovli Tovli left a comment

Choose a reason for hiding this comment

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

Nice work
Please go over the .sh files and ask yourself - if it was written in any other language, would I keep the code as it is now? would it pass your code review if it was ruby/js/go?

@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 2 times, most recently from 31b5d7b to 0d25e50 Compare August 4, 2020 07:56
@sigalsax sigalsax requested a review from orenbm August 4, 2020 08:03
apiVersion: v1
kind: Secret
metadata:
name: test-k8s-secret-two
Copy link
Member

Choose a reason for hiding this comment

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

@sigalsax what's the purpose of this k8s secret? can't we use the one that we already have?

@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 4 times, most recently from 05e6d11 to 2b148ea Compare August 5, 2020 08:00
@sigalsax sigalsax requested a review from orenbm August 5, 2020 08:00
@sigalsax sigalsax marked this pull request as ready for review August 5, 2020 08:01
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch from 2b148ea to e45e7e9 Compare August 5, 2020 08:02
verify_secret_value_in_pod $pod_name "TEST_SECRET" "supersecret"
}

function deploy_chart {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer not having function in the test it self. We can use those functions in other tests as well now or in the future.
For example you always run fill_helm_chart and helm install maybe with different parameters but i prefer this file will be clean.
Moving from clean files to BDD in the future will be easier as well.

@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 5 times, most recently from 65170a5 to b3b4036 Compare August 9, 2020 04:39
@sigalsax sigalsax requested a review from eladkug August 19, 2020 05:32
sigalsax added a commit that referenced this pull request Aug 19, 2020
- Adds integration tests as detailed in [solution design](https://github.com/cyberark/secrets-provider-for-k8s/blob/master/design/milestone_1_2_design_doc.md)
- Adds helper utils needed to build K8s resource and Helm charts integration-tests-job-milestone (#171)
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch from 3d2c675 to 15d208d Compare August 19, 2020 05:40
 - Adds ability to deploy the Secrets Provider via Helm for development environment
 - Updates reload script to support Helm flow
 - Adds cleanup of Helm charts between runs
 - Adds documentation on how to deploy with Helm
- Adds props for Helm tests including create Conjur and K8s/Openshift resources for test use
- Adds ci folder for overriding Helm chart defaults dynamically
sigalsax added a commit that referenced this pull request Aug 19, 2020
- Adds integration tests as detailed in [solution design](https://github.com/cyberark/secrets-provider-for-k8s/blob/master/design/milestone_1_2_design_doc.md)
- Adds helper utils needed to build K8s resource and Helm charts integration-tests-job-milestone (#171)
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 5 times, most recently from 7dcb1c8 to 5490d28 Compare August 19, 2020 09:15
- Adds integration tests as detailed in [solution design](https://github.com/cyberark/secrets-provider-for-k8s/blob/master/design/milestone_1_2_design_doc.md)
- Adds helper utils needed to build K8s resource and Helm charts integration-tests-job-milestone (#171)
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 3 times, most recently from 8cc4ddf to d35a65c Compare August 19, 2020 09:37
if (( $duration >= $retryIntervalMin && $duration <= $retryIntervalMax )); then
echo 0
else
echo 1
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add here the times in logs. If we will fail we need to understand the reason.

@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch 3 times, most recently from 884c2ca to 39db4c6 Compare August 19, 2020 12:32
@sigalsax sigalsax force-pushed the integration-tests-job-milestone branch from 39db4c6 to 7444d4e Compare August 19, 2020 14:01
@codeclimate
Copy link

codeclimate bot commented Aug 19, 2020

Code Climate has analyzed commit 7444d4e and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 86.6% (0.0% change).

View more on Code Climate.

@eladkug eladkug self-requested a review August 19, 2020 14:49
Copy link
Contributor

@eladkug eladkug left a comment

Choose a reason for hiding this comment

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

LGTM from test perspective

@sigalsax sigalsax dismissed stale reviews from abrahamko, orenbm, and Tovli August 20, 2020 05:18

.

@sigalsax sigalsax merged commit b0c20c3 into master Aug 20, 2020
@sigalsax sigalsax deleted the integration-tests-job-milestone branch August 20, 2020 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants