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

Support for externally referenced S3 credentials and quoted plain values #43

Closed
Heiko-san opened this issue Aug 21, 2023 · 7 comments · Fixed by #44
Closed

Support for externally referenced S3 credentials and quoted plain values #43

Heiko-san opened this issue Aug 21, 2023 · 7 comments · Fixed by #44
Assignees
Labels
bug Something isn't working enhancement New feature or request
Milestone

Comments

@Heiko-san
Copy link

Hi,

we created an additional datastore of type etcd using your helm chart clastix/kamaji-etcd.
We successfully set up it with an s3 backup. However in our values.yaml we had to define our password like this (not the real pwd):

  s3:
    #...
    bucket: kamaji/kamaji-etcd-2/
    accessKey: kamaji
    secretKey: "'xY%foo$ba)R'"

This is necessary so that the single quotes get added to the mc command, otherwise shell would try to interpret the password.
Maybe in the containers command, the password should be quoted by default.

We use kamaji v0.3.3 together with cluster-api v1.5.0 with infrastructure-vsphere v1.7.0 and your kamaji plugin control-plane-kamaji v0.3.0.

@prometherion prometherion added the bug Something isn't working label Aug 21, 2023
@prometherion
Copy link
Member

Thanks for the report, @Heiko-san!

I guess we're just missing the | quote helper when interpolating the variable.

I'll raise a PR, it would be great if you could give it a try.

@Heiko-san
Copy link
Author

Heiko-san commented Aug 21, 2023

The change doesn't fix the problem entirely.
The password now gets quoted, but with double quotes, so a $ in the password for example still gets interpreted by the shell.
It has to use single quotes. (which may actually break aswell if you have ' in your passwort, which may be an acceptable edge case, but to be clean, it would need something like | shellescape I guess)

spec:
  containers:
  - command:
    - bash
    - -c
    - |-
      cd /opt/etcd-dump
      if $MC alias set myminio https://s3.url kamaji "Pa$$w0rd" \
      && $MC ping myminio -c 3 -e 3 ; then
        echo -e "\nUploading snapshot(s):"
        $MC cp kamaji-etcd-5_*.db myminio/kamaji/kamaji-etcd-5/
      else
        echo -e "\nERROR: S3 storage could not be configured;\nCheck your S3 URL/Credentials or network connectivity"
        exit 1
      fi
    env:

@Heiko-san
Copy link
Author

Heiko-san commented Aug 21, 2023

or maybe provide the password with an env variable would help?

You also could use a secret then.

@ptx96
Copy link
Contributor

ptx96 commented Aug 21, 2023

or maybe provide the password with an env variable would help?

Hi @Heiko-san

Going to open an Issue to use a previously created secret;
IMHO, is the right way to do this :)

@Heiko-san
Copy link
Author

i think so too, it also isn't time critical, since we found a workaround and are still in proof of concept phase

@prometherion prometherion changed the title BUG mc backup of helm chart needs zu quote password Support for externally referenced S3 credentials and quoted plain values Aug 22, 2023
@prometherion
Copy link
Member

The attached PR is addressing both use cases, such as:

  • plain values, quoted
  • Secret reference

@prometherion prometherion added this to the v0.4.0 milestone Aug 22, 2023
@prometherion prometherion added the enhancement New feature or request label Aug 22, 2023
@Heiko-san
Copy link
Author

Both approaches (secret ref & quoted value) work great in our setup!
Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants