1Password / connect-helm-charts

Official 1Password Helm Charts
https://developer.1password.com
MIT License
93 stars 74 forks source link

Instructions are misleading #60

Closed FabioAntunes closed 3 years ago

FabioAntunes commented 3 years ago

I can raise a PR to update the README but wanted to discuss this first with you folks.

If I have a 1password-credentials.json:

{
  "verifier": {
    "salt": "asd",
    "localHash": "asd"
  },
  "encCredentials": {
    "kid": "asd",
    "enc": "asd",
    "cty": "asd",
    "iv": "asd",
    "data": "asd"
  },
  "version": "2",
  "deviceUuid": "asd",
  "uniqueKey": {
    "alg": "asd",
    "ext": true,
    "k": "asd",
    "key_ops": [
      "encrypt",
      "decrypt"
    ],
    "kty": "oct",
    "kid": "asd"
  }
}

and run this:

helm install connect 1password/connect --set-file connect.credentials=<path/to/1password-credentials.json>

This is going to fail because we are passing a literal json file to the helm chart.

When what we really want is to do something like:

cat <path/to/1password-credentials.json> | base64 > <path/to/op-session>
helm install connect 1password/connect --set-file connect.credentials=<path/to/op-session>
florisvdg commented 3 years ago

Hi Fabio, just to clarify: by saying 'this is going to fail' do you mean 'this has failed for me' or do you mean 'I suspect that this will fail'?

FabioAntunes commented 3 years ago

It's going to fail, at least with this version of the chart. Because the JSON file gets base64 encoded for the secret, but then when it gets injected in the pod, it gets base64 decoded which means the env var OP_SESSION will have the literal JSON.

So we need to base64 the JSON file, before we pass it to the kubernetes secret, which base64 encodes it again.

On Tue, 6 Jul 2021, 08:54 Floris van der Grinten, @.***> wrote:

Hi Fabio, just to clarify: by saying 'this is going to fail' do you mean 'this has failed for me' or do you mean 'I suspect that this will fail'?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/1Password/connect-helm-charts/issues/60#issuecomment-874544164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATNIINHVUMQL62GWA6O5ITTWKZENANCNFSM47XA2ZAQ .

florisvdg commented 3 years ago

The Kubernetes Secret uses stringData, so there's no additional base64 decoding step: https://github.com/1Password/connect-helm-charts/blob/5a7f24d2db7ff06504dfa576824bbc737a43e014/charts/connect/templates/connect-credentials.yaml#L10-L12

So you should just be able to do --set-file connect.credentials=<path/to/1password-credentials.json>. The CI tests also do that.

Let me know if that works for you.

FabioAntunes commented 3 years ago

Hi @florisvdg apologies for wasting your time. You are right the problem was the secrets were modified to data instead of stringData