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

Add --build-arg and --build-secret #50

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fredericrous
Copy link

@fredericrous fredericrous commented Mar 30, 2024

fix #4

usage example

      - name: ✨ Deploy Preview
        uses: superfly/fly-pr-review-apps@1.3.0
        with:
          build_args: |
            COMMIT_SHA=${{ github.sha }}
            OTHER_VARIABLE=true
          build_secrets: |
            SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}

@pedroassumpcao
Copy link

This PR is great and fixes one of my needs as well.

@@ -18,7 +18,9 @@ If you have an existing `fly.toml` in your repo, this action will copy it with a
| `path` | Path to run the `flyctl` commands from. Useful if you have an existing `fly.toml` in a subdirectory. |
| `postgres` | Optional name of an existing Postgres cluster to `flyctl postgres attach` to. |
| `update` | Whether or not to update this Fly app when the PR is updated. Default `true`. |
| `secrets` | Secrets to be set on the app. Separate multiple secrets with a space |
| `secrets` | Secrets to be set on the app at runtime. Separate multiple secrets with a space |
| `build_args` | Optional Docker --build-arg |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we add this option we don't need to pass dockerfile option explicitly anymore ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the answer to your question is yes. I don't have enough context to give you a straight forward good quality answer tho.
my use case is as follow:

I use build_args to set, inside the image, as an environment variable:

  • the commit sha. I use this because I need to tell apart which sourcemap sent to Sentry correspond to which build
  • a flag autosleep. My Remix server checks at runtime if that flag is there and if so it shuts itself down after a while in order to scale the fly machine down

build_secrets I use to set credentials needed by Sentry to upload the sourcemaps

and finally my dockerfile is the same for build and for production but the generated image is different in production than what's used for build because I make use of FROM more than once in the same dockerfile

FROM can appear multiple times within a single Dockerfile to create multiple images or use one build stage as a dependency for another. Simply make a note of the last image ID output by the commit before each new FROM instruction. Each FROM instruction clears any state created by previous instructions.

https://docs.docker.com/reference/dockerfile/#from

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it , thanks

@suddenlyGiovanni
Copy link

This PR is great and fixes one of my needs as well.

I have a use case where I rely on a different npm package registry. It requires a token to operate (read), so I need to provide access to it when the docker images are built. I'm doing that by passing it as a "build_arg" to fly deploy.

This is something that I can do with superfly/flyctl-actions/setup-flyctl@master, but can't with superfly/fly-pr-review-apps@1.2.0

Please, consider it.

@KrisCoulson
Copy link

Would also love to see this get merged. We have a similar problem needing build args for our frontend services and SHA for issue tracking. Currently we have a fork of this repository to add this functionality. Would love to just be able to use the official action.

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.

Ability to set --build-arg
5 participants