Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

CORTX-30141: Remove CORTX secrets custom template file #232

Merged
merged 4 commits into from
May 7, 2022
Merged

CORTX-30141: Remove CORTX secrets custom template file #232

merged 4 commits into from
May 7, 2022

Conversation

keithpine
Copy link
Contributor

Description

Remove the custom template file used for creating the CORTX secret, and generate the secret directly with kubectl create. This reduces the number of template files to manage, in support of merging the cortx-configmap Chart into the "unified" Chart.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds new functionality)
  • Breaking change (bug fix or new feature that breaks existing functionality)
  • Third-party dependency update
  • Documentation additions or improvements
  • Code quality improvements to existing code or test additions/updates

Applicable issues

  • This change is related to an issue: CORTX-30141

How was this tested?

Additional information

Deployed a cluster and performed S3 I/O for each of these:

  • custom passwords in solution.yaml
  • auto-generated passwords
  • an external secret

Examined the created secret and compared it from previous, no unexpected changes seen.

Checklist

  • The change is tested and works locally.
  • New or changed settings in the solution YAML are documented clearly in the README.md file.
  • All commits are signed off and are in agreement with the CORTX Community DCO and CLA policy.

If this change addresses a CORTX Jira issue:

  • The title of the PR starts with the issue ID (e.g. CORTX-XXXXX:)

Use kubectl to create the generic secret directly.

Signed-off-by: Keith Pine <[email protected]>
@keithpine keithpine requested a review from a team as a code owner May 4, 2022 19:48
@cla-bot cla-bot bot added the cla-signed label May 4, 2022
Copy link
Contributor

@walterlopatka walterlopatka left a comment

Choose a reason for hiding this comment

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

lgtm

kubectl_create_secret_cmd="kubectl create -f ${new_secret_gen_file} --namespace=${namespace}"
if ! ${kubectl_create_secret_cmd}; then

if ! kubectl create secret generic "${cortx_secret_name}" \
Copy link
Contributor

Choose a reason for hiding this comment

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

kubectl apply is preferred here as it will not fail if the existing secret name already exists, but it will fail if kubectl create is used and a secret with the same name already exists. It is a valid and expected use case to update the secret with apply as needed.

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
if ! kubectl create secret generic "${cortx_secret_name}" \
if ! kubectl apply secret generic "${cortx_secret_name}" \

Copy link
Contributor Author

@keithpine keithpine May 6, 2022

Choose a reason for hiding this comment

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

It doesn't look like this command exists:

❯ k apply secret generic -f /tmp/foobar.yaml
error: Unexpected args: [secret generic]
See 'kubectl apply -h' for help and examples

If we want to use apply, then I can just close this PR and move the bespoke template file to another location. I was mostly just attempting to get rid of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! Good catch! I was jumping ahead too quickly. For now we can pivot to what you have here. It won't matter much in the near future, so I'm good with what you have above.

@osowski osowski merged commit f376d9f into Seagate:integration May 7, 2022
@keithpine keithpine deleted the CORTX-30141_remove-secret-template branch May 17, 2022 21:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants