fuentesloic / nuxt-stripe

MIT License
90 stars 8 forks source link

feat: make keys editable at runtime #18

Closed sandros94 closed 1 year ago

sandros94 commented 1 year ago

Usually I build my docker images via GitHub Actions, and I try to make sure that any env variable like secrets and keys are available at runtime.

Are there any limitations for not letting both keys editable via NUXT_PUBLIC_* runtimeConfigs?

release.yml ```yml name: New Release on: push: tags: - 'v[0-9]*' env: GHCR_IMAGE: ghcr.io/${{ github.repository }} jobs: create-release: name: Create Release runs-on: ubuntu-latest permissions: contents: write steps: - name: Checkout repository uses: actions/checkout@v3 - name: Create release env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: gh release create "$GITHUB_REF_NAME" --notes "$GITHUB_REF_NAME" build-image: permissions: contents: write packages: write name: Build Image runs-on: ubuntu-latest steps: - name: Checkout repository uses: actions/checkout@v3 - name: Setup QEMU uses: docker/setup-qemu-action@v2 - name: Setup Docker Buildx uses: docker/setup-buildx-action@v2 - name: Cache Docker layers uses: actions/cache@v3 with: path: /tmp/.buildx-cache key: ${{ runner.os }}-buildx-${{ github.sha }} restore-keys: | ${{ runner.os }}-buildx- - name: Extract metadata for Docker image id: meta uses: docker/metadata-action@v4 with: images: ${{ env.GHCR_IMAGE }} tags: | type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} - name: Login to GHCR uses: docker/login-action@v2 if: env.GHCR_IMAGE with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push uses: docker/build-push-action@v4 with: context: . file: ./dockerfile.nuxt tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} platforms: linux/amd64 # platforms: linux/amd64,linux/arm64 ---> Disabled because not used and increased the build time by 10x push: true cache-from: type=local,src=/tmp/.buildx-cache cache-to: type=local,dest=/tmp/.buildx-cache-new # Temp fix: # https://github.com/docker/build-push-action/issues/252 # https://github.com/moby/buildkit/issues/1896 - name: Move cache run: | rm -rf /tmp/.buildx-cache mv /tmp/.buildx-cache-new /tmp/.buildx-cache ```
flozero commented 1 year ago

Hey @Sandros94 thanks for the message :) Are you referring the api key and publishable key ? If yes the api can't be available in public as it is suppose to be secret. and public variables can be accessible by the final user.

sandros94 commented 1 year ago

Are you referring the api key and publishable key ?

Yes

the api can't be available in public as it is suppose to be secret

Ah right, I didn't choose the right wording:

Im talking about the runtime configs, that lets you change variables at runtime, both public and secret ones.

In this case yes, the publishable key will be under NUXT_PUBLIC_STRAPI_PUBLISHABLEKEY while the api key is under NUXT_STRAPI_APIKEY. This later one is only accessible by the server.

While looking at the code I didn't see them being used, but I wasn't sure if it was because of a problem.

sandros94 commented 1 year ago

So, I was about to open up a PR, but I've noticed that the runtimeConfig is already configured:

https://github.com/fuentesloic/nuxt-stripe/blob/23c74938a3c10173f07c451dba3fbb20fb493931/src/module.ts#L58-L67

But just above that there are these lines that prevent the build from succeeding, making the runtimeConfig useless(?):

https://github.com/fuentesloic/nuxt-stripe/blob/23c74938a3c10173f07c451dba3fbb20fb493931/src/module.ts#L48-L56

Why there must be a check for the availability of those two? Wouldn't it be easier just to check if they are defined via runtimeConfig and then if not just disable both stripe and stripe.js?

sandros94 commented 1 year ago

Ok I did test to delete those checks, I imagine that there will be some hydration issues if no keys are provided.

I'm opening a PR to discuss this a bit more.

flozero commented 1 year ago

Oh yeah. We can think of a way to disable this check also. If its something that is useful. Would this help you ?

flozero commented 1 year ago

Hey I have done a PR that should handle the behavior you are expecting. It will be loaded only the first time you will use it. Check the PR. This will be merge this week. It will create a breaking change as we are going to harmonize the api config

sandros94 commented 1 year ago

Hey I have done a PR that should handle the behavior you are expecting. It will be loaded only the first time you will use it. Check the PR. This will be merge this week. It will create a breaking change as we are going to harmonize the api config

I'll respond in the PRs #19 and #22