Issue #439 Strip out double quotes in ARG value

[cvgw]

Fixes #439

Description

Strip out double quotes enclosing ARG value after parsing Dockerfile. This brings Kaniko into parity with Docker for handling ARG values. This prevents failures where things like image names would not parse correctly due to the double quotes remaining.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes unit tests
• Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

• The code flow looks good.
• Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

- Strip double quotes enclosing the value of ARG commands before substituting those values in during shell expansion

[tejal29]

We can add table driven unit tests here for this function only

1. value like"foo"
2. For empty value "" ? What happens then?
   maybe we shd keep quotes as is then?
3. value foo
4. invalid value "foo shd probably error out?

[tejal29]

Some other test cases to add would be
5. When metaArgs is empty
6. Multiple mataArgs instructions have "". (All get resolved)

[priyawadhwa]

just something to consider, something like \"foo\" would probably need to maintain the quotes

[cvgw]

We can add table driven unit tests here for this function only

1. value like"foo"
2. For empty value "" ? What happens then?
   maybe we shd keep quotes as is then?
3. value foo
4. invalid value "foo shd probably error out?

2. Docker seems to strip out the quotes and treat it as a valid, but blank string
3. Yup, Docker also seems to error when either quote isn't matched

We also might want to handle single quotes as docker seems to handle those as well

[cvgw]

just something to consider, something like \"foo\" would probably need to maintain the quotes

Ya, it seems that Docker does not remove the quotes in that case which will result in an error for things like image name

[priyawadhwa]

I didn't look thoroughly at this, but this may be what docker uses to resolve quotes, which we could potentially use as well to ensure we respond to this as docker does

[cvgw]

I didn't look thoroughly at this, but this may be what docker uses to resolve quotes, which we could potentially use as well to ensure we respond to this as docker does

@priyawadhwa kaniko is currently using that same package to do the variable expansion. I was surprised that it doesn't seem to be handling the quotes, but I didn't see anything in the documentation referring to that. I did notice that in a recent version (later than what kaniko seems to be using) that RawQuotes field was added. Perhaps that is related?

[priyawadhwa]

I did notice that in a recent version (later than what kaniko seems to be using) that RawQuotes field was added. Perhaps that is related?

Yah that sounds like it could be! We could definitely bump up the version if you think that would work

[cvgw]

I did notice that in a recent version (later than what kaniko seems to be using) that RawQuotes field was added. Perhaps that is related?

Yah that sounds like it could be! We could definitely bump up the version if you think that would work

Unfortunately doesn't seem to be related

package buildkit_quotes_test
  
  import (
          "testing"
  
          "github.com/moby/buildkit/frontend/dockerfile/parser"
          "github.com/moby/buildkit/frontend/dockerfile/shell"
  )
  
  func TestWithDoubleQuotes(t *testing.T) {
          envs := []string{"MEOW=purr", "VAL=\"World\""}
  
          lex := shell.NewLex(parser.DefaultEscapeToken)
          val, err := lex.ProcessWord("Hello ${VAL}", envs)
          if err != nil {
                  t.Error(err)
          }   
  
          if val != "Hello World" {
                  t.Errorf("Expected Hello World but got %s", val)
          }   
  }
  
  func TestWithDoubleQuotesRawQuotesTrue(t *testing.T) {
          envs := []string{"MEOW=purr", "VAL=\"World\""}
  
          lex := shell.NewLex(parser.DefaultEscapeToken)
          lex.RawQuotes = true
  
          val, err := lex.ProcessWord("Hello ${VAL}", envs)
          if err != nil {
                  t.Error(err)
          }   
  
          if val != "Hello World" {
                  t.Errorf("Expected Hello World but got %s", val)
          }   
  }
  // "Hello "World""