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

fix: credential secret does not works if set from values #196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grhawk
Copy link

@grhawk grhawk commented May 25, 2024

Same as #113 with verified commit.

@jillianwilson
Copy link
Contributor

Hey @grhawk ! Thanks for creating the new PR! Looks like there are some merge conflicts here. Do you mind taking a look at those?

@grhawk
Copy link
Author

grhawk commented Jun 5, 2024

@jillianwilson sorry, I don't remember out of my head what I did there since it is about 2y old and I don't have much time to re-look into it from scratch :)

@stewartmalik
Copy link

I'm actually not sure that this one is required, 1Password/connect#62 seems to suggest that connect requires the value to be doubly base64 encoded.

I have run the chart test suite successfully without these changes and everything worked as expected, I was able to retrieve my secret value.

@grhawk
Copy link
Author

grhawk commented Jul 17, 2024

I think a doubly encoded secret looks more like a workaround than a solution and generates annoying effects. For example, all tools that allow to see secrets will not work properly since they expect the secret to be encoded only once.

@grhawk
Copy link
Author

grhawk commented Jul 17, 2024

@jillianwilson I think you already fixed the conflicts in the previous MR. See adcbeff

@smauermann
Copy link

Bump, I would love for this to be merged. Without a fix to the double base64 encoding situation it is really hard bootstrapping onepassword connect in a GitOps environment. 🙏

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.

4 participants