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

Make creating the secret optional #160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

gaima8
Copy link

@gaima8 gaima8 commented Apr 8, 2021

Same as purestorage/helm-charts#258 but for the newer release.

My aim is to have the flux helm-controller install the chart, and it's impractical to pass the API tokens securely for the chart to create the secret.
A sealed-secret solves the problem of getting the values in but the chart will at best overwrite the unsealed secret.

I am very much open to ideas on how to handle the validation of arrays better.

Copy link
Contributor

@Pure-AdamuKaapan Pure-AdamuKaapan left a comment

Choose a reason for hiding this comment

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

@sdodsley this request seems reasonable to me, we already function fine with the secret missing (we just kinda sit there until it appears). Can you think of any issues this might cause?

@gaima8 I left a few comments if you'd be so kind as to address them, they're mostly just small doc changes. Thanks for your contribution!

@@ -204,7 +204,7 @@ The following table lists the configurable parameters and their default values.
| `flashblade.exportRules` | NFS Export Rules. Please refer the FlashBlade User Guide. | "" |
| `flashblade.snapshotDirectoryEnabled` | Enable/Disable FlashBlade snapshots | `false` |
| `orchestrator.name` | Orchestrator type, such as openshift, k8s | `k8s` |
| *`arrays` | Array list of all the backend FlashArrays and FlashBlades | must be set by user, see an example below |
| *`arrays` | Array list of all the backend FlashArrays and FlashBlades | may be set by user, see two examples below |
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] I'd just change it to "may be set by user, see examples below" (remove "two")

@@ -250,6 +250,8 @@ The following table lists the configurable parameters and their default values.

*Examples:

1. Helm values
To have helm create and manage the secret holding the API tokens use the following in your values file.
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] capitalize Helm, and end the sentence in a colon instead of period

@@ -266,6 +268,40 @@ arrays:
NFSEndPoint: "1.2.3.9"
```

2. Manual secret
If you wish to manage the secret holding the API tokens yourself do the following;
Create a kubernetes secret called `pure-provisioner-secret` with a single key `pure.json` containing json formatted like so
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Create a kubernetes secret called `pure-provisioner-secret` with a single key `pure.json` containing json formatted like so
Create a Kubernetes secret called `pure-provisioner-secret` in the same namespace as your PSO installation with a single key `pure.json` containing json formatted like so:

pure-pso/README.md Show resolved Hide resolved
@sdodsley
Copy link
Contributor

@Pure-AdamuKaapan Is this change going to mess up the helm schema?

@Pure-AdamuKaapan
Copy link
Contributor

@sdodsley I think this would make it so that arrays is no longer required (obviously). I think this would still validate that the array structs are well formed if they're there but we should confirm that locally to be sure

@gaima8
Copy link
Author

gaima8 commented Apr 20, 2021

Hi, requested changes to the README made.

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.

3 participants