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

Build Args with comma in value throws "invalid build args" error #1082

Open
samharnack opened this issue Jul 17, 2022 · 5 comments
Open

Build Args with comma in value throws "invalid build args" error #1082

samharnack opened this issue Jul 17, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@samharnack
Copy link

Please only report specific issues with flyctl behavior. Anything like a support request for your application should go to https://community.fly.io. More people watch that space and can help you faster!

Describe the bug
Briefly, describe what broke and provide the following details:

Same issue a #520

Running fly deploy --build-arg SOME_BUILD_ARG="a,b" throws the following:

==> Verifying app config
--> Verified app config
==> Building image
Error failed to fetch an image or build from source: invalid build args: 'b': must be in the format NAME=VALUE

I've also tried:

fly deploy --build-arg SOME_BUILD_ARG=a,b  
fly deploy --build-arg SOME_BUILD_ARG="a\,b"
fly deploy --build-arg SOME_BUILD_ARG=a\,b

export SOME_BUILD_ARG_VALUE="a,b"
fly deploy --build-arg SOME_BUILD_ARG=$SOME_BUILD_ARG_VALUE
  • Operating system
  • fly version

fly v0.0.353 darwin/arm64 Commit: a6e61fa BuildDate: 2022-07-12T11:36:55Z

** Paste your fly.toml

# fly.toml file generated for APPNAME on 2022-07-17T08:16:59-07:00

app = "APPNAME"
kill_signal = "SIGINT"
kill_timeout = 5
processes = []

[experimental]
  allowed_public_ports = []
  auto_rollback = true

[[services]]
  http_checks = []
  internal_port = 8080
  processes = ["app"]
  protocol = "tcp"
  script_checks = []
  [services.concurrency]
    hard_limit = 25
    soft_limit = 20
    type = "connections"

  [[services.ports]]
    force_https = true
    handlers = ["http"]
    port = 80

  [[services.ports]]
    handlers = ["tls", "http"]
    port = 443

  [[services.tcp_checks]]
    grace_period = "1s"
    interval = "15s"
    restart_limit = 0
    timeout = "2s"

** Command output: **

$ fly deploy --build-arg SOME_BUILD_ARG="a,b"              
                
==> Verifying app config
--> Verified app config
==> Building image
Error failed to fetch an image or build from source: invalid build args: 'b': must be in the format NAME=VALUE
@samharnack samharnack added the bug Something isn't working label Jul 17, 2022
@galaxyfeeder
Copy link

Having the exact same issue with environment variables.

fly deploy --env TEST="a:b,c:d"
==> Verifying app config
Error failed parsing environment: 'c:d': must be in the format NAME=VALUE

@galaxyfeeder
Copy link

I've just digged a little bit more on this, and as far as I understand, this issue comes from the way is used the library pflag which handles the command arguments and options.

Right now, both build-arg and env flags are treated as an StringSlice which means that pflag will try to convert --ss="v1,v2" --ss="v3" into []string{"v1", "v2", "v3"}.

From my point of view, what would solve this issue is to use a StringArray which will not introduce this behaviour, and would be as easy as changing the argument type.

However, this could mean that users that are currently relying on using this comma will need to add multiple --env or multiple --build-arg arguments, but at the same time will allow other user like myself or @samharnack to be able to pass arguments which their value includes a comma.

What do you think @michaeldwan @jsierles?

Would a PR changing this be useful?

@lillianberryfly
Copy link
Collaborator

fly deploy --env "\"TEST=a:b,c:d\"" works, but I agree this should be fixed properly

@galaxyfeeder
Copy link

galaxyfeeder commented Dec 9, 2022

I didn't tried that one, thanks for posting it, I've tried another combination of simple, and double ticks between wrapping the value or the key.

If there is a way to allow this behaviour, it might not make sense to potentially break other users usage. But fmpov, a nicer solution would be better and easier to understand for everyone.

@redjonzaci
Copy link
Contributor

I just tested both fly deploy --env and fly deploy --build-arg.
Both worked and wanted to let you know @samharnack @galaxyfeeder.
Could you confirm it works for you too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants