feat: support BW_APPID #82
Conversation
|
I still need to test this change. |
|
Thanks for this and your other PR! :) When it's tested, let us know! Also, be sure to bump the helm chart version here by a patch version: |
|
I realized that this could likely also be accomplished by storing the ~/.config/Bitwarden\ CLI/` in a PVC. I'm honestly not sure which approach I would choose. On the one hand, the PVC results in less data having to be transferred but results in all of the vault contents being persisted to disk. |
|
@cloudymax, can you also take a peek at this one? It seems like a dream come true, because we get so many email notifications with how often we tear down and re-create test clusters 😵
I feel like accepting a config file is ok, as some users may have robust encrypted drives with otherwise good security. And those that don't, may just benefit from a minor warning in the values.yaml? 🤷 |
|
Also, is |
doesnt seem to be based on the api spec: https://bitwarden.com/help/vault-management-api/ probably just brute-force regexing a field it would normally auto-generate per session but idk |
|
I identified that the
|
|
That makes sense, I'll try to do a test-run with the changes and verify I can get it working on my end as well. Should be able to merge soon 👍 |
|
|
||
| env: | ||
| REGISTRY: registry.hub.docker.com | ||
| IMAGE_NAME: ${{ github.repository_owner }}/bweso |
There was a problem hiding this comment.
| IMAGE_NAME: ${{ github.repository_owner }}/bweso | |
| IMAGE_NAME: jessebot/bweso |
The docker image is released under the username and profile called jessebot on docker hub, not small-hack, so this would break if not updated.
| - name: Login to GitHub Container Registry | ||
| if: ${{ env.REGISTRY == 'ghcr.io' }} | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| registry: ${{ env.REGISTRY }} | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Not sure we need this right now.
| @@ -48,11 +61,11 @@ jobs: | |||
| # build from the ./docker directory in this repo | |||
| context: docker | |||
| platforms: | | |||
| linux/amd64 | |||
| linux/amd64,linux/arm64 | |||
There was a problem hiding this comment.
please put this in a separate PR so that we can roll back if it breaks anything.
|
Please bump the chart version and the appVersion in Chart.yaml, so that we can release the new docker image with the new tag and the new helm release has your changes. Please also see our comments on the docker build and push workflow, as these changes would break our existing release process. |
|
The changes are good, but doing proper testing is tricky because there's a few PRs worth of work combined here. It would be ideal to separate out the work into the following:
If we can get that done, this should be smooth-sailing. Right now though I would need to build and release your container + fork and modify our argo appsets then rebuild a new cluster etc... to put it through its paces properly. Much easier to just chop it up into bite-sized PRs. |
This adds the ability to specify a bitwarden appId that can be used to consistently identify the service running. Refs: #81
|
I removed all but the changes to the dockerfile for this PR. My other changes were me attempting to build the image using github actions and I just gave up and pushed an image directly to my docker hub. It's been a while since I tried to do anything with docker hub and github actions. I apologize for the confusing changes. I'll open a new PR for the arm64 support and for supporting the BW_APPID in the chart. |
This changeset adds support for the BW_APPID environment variable so that the pod can be uniquely identified making it easier to identify when a new login is detected.
Refs: #81