Add helm chart #273
Add helm chart #273
Conversation
bhavanapalle2023
requested review from
tam7t,
sbadame and
amitmodak
as code owners
README.md
Outdated
| kubectl apply -f deploy/provider-gcp-plugin.yaml | ||
| # if you want to use helm | ||
| # helm upgrade --install secrets-store-csi-driver-provider-gcp charts/secrets-store-csi-driver-provider-gcp | ||
| $ export PROJECT_ID=<your gcp project> |
There was a problem hiding this comment.
We should not merge this change in instructions unless the package is indeed available in the public repo.
There was a problem hiding this comment.
done, back to original ReadMe
scripts/cloudbuild-dev.yaml
Outdated
| @@ -15,10 +15,30 @@ | |||
| # Usage: from the root directory run: | |||
| # | |||
| # $ gcloud builds submit --config scripts/cloudbuild-dev.yaml | |||
| timeout: 1200s | |||
| timeout: 3600s #increasing timeout else build crashes | |||
There was a problem hiding this comment.
Nit: no need for the comment
| @@ -27,6 +47,7 @@ steps: | |||
| '--build-arg', | |||
| 'VERSION=$TAG_NAME', | |||
| '-t', | |||
| 'gcr.io/$PROJECT_ID/secrets-store-csi-driver-provider-gcp:$TAG_NAME', | |||
| 'asia-east1-docker.pkg.dev/$PROJECT_ID/secrets-store-csi-driver-provider-gcp/provider-image:$TAG_NAME', | |||
There was a problem hiding this comment.
Why is this change required?
There was a problem hiding this comment.
As Container Registry is Depreciating, Made helm image repo, helm chart repo and provider image in the Artifact registry
| @@ -23,7 +23,7 @@ spec: | |||
| serviceAccountName: {{ include "secrets-store-csi-driver-provider-gcp.serviceAccountName" . }} | |||
| containers: | |||
| - name: provider | |||
| image: "{{ .Values.image.repository }}@{{ .Values.image.hash }}" | |||
| image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" | |||
There was a problem hiding this comment.
Using a hash is considered more secure and we should continue to reference by hash.
| @@ -4,3 +4,7 @@ description: A Helm chart to install Google Secret Manager Provider for Secret S | |||
| type: application | |||
| version: 0.1.0 | |||
There was a problem hiding this comment.
I think its okay to keep the version same as appVersion
| @@ -6,8 +6,8 @@ serviceAccount: | |||
|
|
|||
| image: | |||
| repository: us-docker.pkg.dev/secretmanager-csi/secrets-store-csi-driver-provider-gcp/plugin | |||
| tag: v1.2.0 | |||
There was a problem hiding this comment.
Same as above, please continue to use hash references.
| '--build-arg', | ||
| 'VERSION=$TAG_NAME', | ||
| '-t', | ||
| 'asia-east1-docker.pkg.dev/$PROJECT_ID/secrets-store-csi-driver-provider-gcp/helm-image:$TAG_NAME', |
There was a problem hiding this comment.
Why is this change required?
There was a problem hiding this comment.
To create helm docker image, As there are no publicaly available helm image. Used Artifact Registry for Repo.
scripts/cloudbuild-dev.yaml
Outdated
| '--push', | ||
| './helm'] | ||
| - name: 'asia-east1-docker.pkg.dev/$PROJECT_ID/secrets-store-csi-driver-provider-gcp/helm-image:$TAG_NAME' | ||
| args: ['dependency', 'update', './charts/secrets-store-csi-driver-provider-gcp'] |
There was a problem hiding this comment.
Nit: it might be good to put each arg on a new line.
scripts/build.sh
Outdated
| @@ -19,6 +19,11 @@ set -o pipefail # Check the exit code of *all* commands in a pipeline | |||
| set -o nounset # Error if accessing an unbound variable | |||
| set -x # Print each command as it is run | |||
|
|
|||
| set +e | |||
| # Creating a new repository secrets-store-csi-driver-provider-gcp | |||
| gcloud artifacts repositories create secrets-store-csi-driver-provider-gcp --repository-format=docker --location=asia-east1 | |||
There was a problem hiding this comment.
nit: inconsistent spacing between keywords, is that intended (2 files)?
There was a problem hiding this comment.
Made the space consistent between the keywords in both the files
| os.Exit(1) | ||
| } | ||
| // Create the helm registry login command | ||
| helmCmd := exec.Command("helm", "registry", "login", "-u", "oauth2accesstoken", "--password-stdin", "https://asia-east1-docker.pkg.dev") |
There was a problem hiding this comment.
Can you confirm if the usage of asia-east1 across this commit is just a choice of region & it can be substituted with a different supported region if required?
If yes, can this be made a constant which is referred everywhere?
There was a problem hiding this comment.
+1, we should use gcr.io instead of regional endpoints wherever possible.
| @@ -19,6 +19,11 @@ set -o pipefail # Check the exit code of *all* commands in a pipeline | |||
| set -o nounset # Error if accessing an unbound variable | |||
| set -x # Print each command as it is run | |||
|
|
|||
| set +e | |||
| # Creating a new repository secrets-store-csi-driver-provider-gcp | |||
| gcloud artifacts repositories create secrets-store-csi-driver-provider-gcp --repository-format=docker --location=asia-east1 | |||
There was a problem hiding this comment.
Why is location asia-east1?
| os.Exit(1) | ||
| } | ||
| // Create the helm registry login command | ||
| helmCmd := exec.Command("helm", "registry", "login", "-u", "oauth2accesstoken", "--password-stdin", "https://asia-east1-docker.pkg.dev") |
There was a problem hiding this comment.
+1, we should use gcr.io instead of regional endpoints wherever possible.
| @@ -86,7 +86,7 @@ spec: | |||
| serviceAccountName: secrets-store-csi-driver-provider-gcp | |||
| containers: | |||
| - name: provider | |||
| image: gcr.io/$PROJECT_ID/secrets-store-csi-driver-provider-gcp:$GCP_PROVIDER_SHA | |||
| image: asia-east1-dev.pkg/$PROJECT_ID/secrets-store-csi-driver-provider-gcp/provider-image:$GCP_PROVIDER_SHA | |||
There was a problem hiding this comment.
Why has the name of the image changed?
No description provided.