Skip to content

Commit

Permalink
cmd/entrypoint: do not interpret anything after --
Browse files Browse the repository at this point in the history
The way the `flag` package works, it can eat the flag
"terminator" (aka the double dash `--`). This means that, in some very
specific cases — where the first item after the `--` is also a
subcommand, it would execute the entrypoint subcommand instead of the
actual command.

For example :

```
$ /ko-app/entrypoint -- init a b
$ /ko-app/entrypoint init a b
```

This is fixed by making sure we remove anything after `--` for the
subcommand processing. And then we pass the rest (after `--`) to the
entrypointer to be executed.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
  • Loading branch information
vdemeester authored and tekton-robot committed Jul 7, 2022
1 parent 9b4bba2 commit da91bf3
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 4 deletions.
38 changes: 38 additions & 0 deletions cmd/entrypoint/args.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright 2022 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

func extractArgs(initialArgs []string) ([]string, []string) {
commandArgs := []string{}
args := initialArgs
if len(initialArgs) == 0 {
return args, commandArgs
}
// Detect if `--` is present, if it is, parse only the one before.
terminatorIndex := -1
for i, a := range initialArgs {
if a == "--" {
terminatorIndex = i
break
}
}
if terminatorIndex > 0 {
commandArgs = initialArgs[terminatorIndex+1:]
args = initialArgs[:terminatorIndex]
}
return args, commandArgs
}
67 changes: 67 additions & 0 deletions cmd/entrypoint/args_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
Copyright 2022 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
"testing"

"github.com/google/go-cmp/cmp"
)

func TestExtractArgs(t *testing.T) {
for _, c := range []struct {
desc string
args []string
expectedArgs []string
expectedCommandArgs []string
}{{
desc: "empty arguments",
args: []string{},
expectedArgs: []string{},
expectedCommandArgs: []string{},
}, {
desc: "with args",
args: []string{"--foo", "bar", "a", "b", "c"},
expectedArgs: []string{"--foo", "bar", "a", "b", "c"},
expectedCommandArgs: []string{},
}, {
desc: "with terminator on its own",
args: []string{"--foo", "bar", "--"},
expectedArgs: []string{"--foo", "bar"},
expectedCommandArgs: []string{},
}, {
desc: "with terminator",
args: []string{"--foo", "bar", "--", "a", "b", "c"},
expectedArgs: []string{"--foo", "bar"},
expectedCommandArgs: []string{"a", "b", "c"},
}, {
desc: "with args and terminator",
args: []string{"--foo", "bar", "baz", "--", "a", "b", "c"},
expectedArgs: []string{"--foo", "bar", "baz"},
expectedCommandArgs: []string{"a", "b", "c"},
}} {
t.Run(c.desc, func(t *testing.T) {
args, commandArgs := extractArgs(c.args)
if d := cmp.Diff(c.expectedArgs, args); d != "" {
t.Errorf("args: diff(-want,+got):\n%s", d)
}
if d := cmp.Diff(c.expectedCommandArgs, commandArgs); d != "" {
t.Errorf("commandArgs: diff(-want,+got):\n%s", d)
}
})
}
}
15 changes: 11 additions & 4 deletions cmd/entrypoint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,16 @@ func main() {
gitcreds.AddFlags(flag.CommandLine)
dockercreds.AddFlags(flag.CommandLine)

flag.Parse()

if err := subcommands.Process(flag.Args()); err != nil {
// Split args with `--` for the entrypoint and what it should execute
args, commandArgs := extractArgs(os.Args[1:])

// We are using the global variable flag.CommandLine here to be able
// to define what args it should parse.
// flag.Parse() does flag.CommandLine.Parse(os.Args[1:])
if err := flag.CommandLine.Parse(args); err != nil {
os.Exit(1)
}
if err := subcommands.Process(flag.CommandLine.Args()); err != nil {
log.Println(err.Error())
switch err.(type) {
case subcommands.SubcommandSuccessful:
Expand Down Expand Up @@ -123,7 +130,7 @@ func main() {
}

e := entrypoint.Entrypointer{
Command: append(cmd, flag.Args()...),
Command: append(cmd, commandArgs...),
WaitFiles: strings.Split(*waitFiles, ","),
WaitFileContent: *waitFileContent,
PostFile: *postFile,
Expand Down
27 changes: 27 additions & 0 deletions examples/v1beta1/taskruns/5080-entrypoint-init-regression.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
generateName: tkn-arg-test-
spec:
params:
- name: ARGS
value:
- init
- foo
- bar
taskSpec:
description: >-
Test consuming args, acts as a regression test for bug 5080
params:
- name: ARGS
description: The terraform cli commands to tun
type: array
default:
- "--help"
steps:
- name: echo-cli
image: registry.access.redhat.com/ubi9/ubi-minimal:9.0.0-1580
workingDir: /tekton/home
args:
- "$(params.ARGS)"
command: ["echo"]

0 comments on commit da91bf3

Please sign in to comment.