Skip to content

Commit 584b527

Browse files
Scotttekton-robot
authored andcommitted
Encode scripts as base 64 to avoid k8s mangling "$$"
Kubernetes replaces instances of "$$" in container args fields with "$". This can muck up the contents of script fields because scripts are passed into a TaskRun Pod as an arg to an init container. Prior to this commit we [tried to prevent the replacement](#3888) from happening by: 1. putting scripts into annotations on a pod and projecting them using downward API - con: the max size of the annotations map is capped to ~250kB. The aggregate size of all scripts in a single Task therefore becomes constrained by this. Any other systems using annotations will reduce the available headroom. Backwards incompatible. 2. replacing instances of "$$" in scripts with "$$$$" for k8s to then process back to "$$" - con: k8s doesn't actually process _all_ instances of "$$". For example, if you write an arg with format "echo $(eval \$$foo)" then k8s will see the first "$(", assume it's a variable reference, and pass it through verbatim. So user's scripts with bash variable become broken by tekton's new replacement. Backwards incompatible. This commit takes a third approach, proposed by @MartinKanters, encoding scripts as base64 in the controller and then having them decoded in the init container. This bypasses Kubernetes' args processing completely because dollar signs aren't used in base64 encodings. It also doesn't introduce a backwards-incompatible limit to the aggregate script size. And it doesn't mangle existing bash scripts with variable replacements. The most noticeable trade-offs we now make are: 1. Tiny scripts can be up to 300% bigger, but as scripts get longer the max increase gets closer to 133%. 2. Also the TaskRun's `initContainer` YAML is a bit less human readable: ``` initContainers: - args: - -c - | tmpfile="/tekton/scripts/script-0-f8fmf" touch ${tmpfile} && chmod +x ${tmpfile} cat > ${tmpfile} << '_EOF_' IyEvYmluL3NoCnNldCAteGUKZWNobyAibm8gc2hlYmFuZyI= _EOF_ /tekton/tools/entrypoint decode-script "${tmpfile}" ``` The entrypoint is extended to decode base64 files so that the `shellImage` (which is used to write scripts to disk for Step containers) is not required to package a `base64` binary.
1 parent ccf723a commit 584b527

File tree

13 files changed

+642
-101
lines changed

13 files changed

+642
-101
lines changed

cmd/entrypoint/main.go

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ package main
1818

1919
import (
2020
"flag"
21-
"io"
2221
"log"
2322
"os"
2423
"os/exec"
2524
"strings"
2625
"syscall"
2726
"time"
2827

28+
"github.com/tektoncd/pipeline/cmd/entrypoint/subcommands"
2929
"github.com/tektoncd/pipeline/pkg/credentials"
3030
"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
3131
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
@@ -45,25 +45,6 @@ var (
4545

4646
const defaultWaitPollingInterval = time.Second
4747

48-
func cp(src, dst string) error {
49-
s, err := os.Open(src)
50-
if err != nil {
51-
return err
52-
}
53-
defer s.Close()
54-
55-
// Owner has permission to write and execute, and anybody has
56-
// permission to execute.
57-
d, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE, 0311)
58-
if err != nil {
59-
return err
60-
}
61-
defer d.Close()
62-
63-
_, err = io.Copy(d, s)
64-
return err
65-
}
66-
6748
func main() {
6849
// Add credential flags originally introduced with our legacy credentials helper
6950
// image (creds-init).
@@ -72,17 +53,14 @@ func main() {
7253

7354
flag.Parse()
7455

75-
// If invoked in "cp mode" (`entrypoint cp <src> <dst>`), simply copy
76-
// the src path to the dst path. This is used to place the entrypoint
77-
// binary in the tools directory, without requiring the cp command to
78-
// exist in the base image.
79-
if len(flag.Args()) == 3 && flag.Args()[0] == "cp" {
80-
src, dst := flag.Args()[1], flag.Args()[2]
81-
if err := cp(src, dst); err != nil {
82-
log.Fatal(err)
56+
if err := subcommands.Process(flag.Args()); err != nil {
57+
log.Println(err.Error())
58+
switch err.(type) {
59+
case subcommands.SubcommandSuccessful:
60+
return
61+
default:
62+
os.Exit(1)
8363
}
84-
log.Println("Copied", src, "to", dst)
85-
return
8664
}
8765

8866
// Copy credentials we're expecting from the legacy credentials helper (creds-init)

cmd/entrypoint/subcommands/cp.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
Copyright 2020 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package subcommands
18+
19+
import (
20+
"io"
21+
"os"
22+
)
23+
24+
const CopyCommand = "cp"
25+
26+
// Owner has permission to write and execute, and anybody has
27+
// permission to execute.
28+
const dstPermissions = 0311
29+
30+
// cp copies a files from src to dst.
31+
func cp(src, dst string) error {
32+
s, err := os.Open(src)
33+
if err != nil {
34+
return err
35+
}
36+
defer s.Close()
37+
38+
d, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE, dstPermissions)
39+
if err != nil {
40+
return err
41+
}
42+
defer d.Close()
43+
44+
_, err = io.Copy(d, s)
45+
return err
46+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
Copyright 2020 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package subcommands
18+
19+
import (
20+
"errors"
21+
"io/ioutil"
22+
"os"
23+
"path/filepath"
24+
"testing"
25+
)
26+
27+
func TestCp(t *testing.T) {
28+
tmp, err := ioutil.TempDir("", "cp-test-*")
29+
if err != nil {
30+
t.Fatalf("error creating temp directory: %v", err)
31+
}
32+
defer os.RemoveAll(tmp)
33+
src := filepath.Join(tmp, "foo.txt")
34+
dst := filepath.Join(tmp, "bar.txt")
35+
36+
if err = ioutil.WriteFile(src, []byte("hello world"), 0700); err != nil {
37+
t.Fatalf("error writing source file: %v", err)
38+
}
39+
40+
if err := cp(src, dst); err != nil {
41+
t.Errorf("error copying: %v", err)
42+
}
43+
44+
info, err := os.Lstat(dst)
45+
if err != nil {
46+
t.Fatalf("error statting destination file: %v", err)
47+
}
48+
49+
if info.Mode().Perm() != dstPermissions {
50+
t.Errorf("expected permissions %#o for destination file but found %#o", dstPermissions, info.Mode().Perm())
51+
}
52+
}
53+
54+
func TestCpMissingFile(t *testing.T) {
55+
tmp, err := ioutil.TempDir("", "cp-test-*")
56+
if err != nil {
57+
t.Fatalf("error creating temp directory: %v", err)
58+
}
59+
defer os.RemoveAll(tmp)
60+
src := filepath.Join(tmp, "doesnt-exist.txt")
61+
dst := filepath.Join(tmp, "bar.txt")
62+
err = cp(src, dst)
63+
if err == nil {
64+
t.Errorf("unexpected success copying missing file")
65+
}
66+
if !errors.Is(err, os.ErrNotExist) {
67+
t.Errorf(`expected "file does not exist" error but received %v`, err)
68+
}
69+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
Copyright 2020 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package subcommands
18+
19+
import (
20+
"bytes"
21+
"encoding/base64"
22+
"fmt"
23+
"io"
24+
"io/ioutil"
25+
"os"
26+
)
27+
28+
const DecodeScriptCommand = "decode-script"
29+
30+
// decodeScript rewrites a script file from base64 back into its original content from
31+
// the Step definition.
32+
func decodeScript(scriptPath string) error {
33+
decodedBytes, permissions, err := decodeScriptFromFile(scriptPath)
34+
if err != nil {
35+
return fmt.Errorf("error decoding script file %q: %w", scriptPath, err)
36+
}
37+
err = ioutil.WriteFile(scriptPath, decodedBytes, permissions)
38+
if err != nil {
39+
return fmt.Errorf("error writing decoded script file %q: %w", scriptPath, err)
40+
}
41+
return nil
42+
}
43+
44+
// decodeScriptFromFile reads the script at scriptPath, decodes it from
45+
// base64, and returns the decoded bytes w/ the permissions to use when re-writing
46+
// or an error.
47+
func decodeScriptFromFile(scriptPath string) ([]byte, os.FileMode, error) {
48+
scriptFile, err := os.Open(scriptPath)
49+
if err != nil {
50+
return nil, 0, fmt.Errorf("error reading from script file %q: %w", scriptPath, err)
51+
}
52+
defer scriptFile.Close()
53+
54+
encoded := bytes.NewBuffer(nil)
55+
if _, err = io.Copy(encoded, scriptFile); err != nil {
56+
return nil, 0, fmt.Errorf("error reading from script file %q: %w", scriptPath, err)
57+
}
58+
59+
fileInfo, err := scriptFile.Stat()
60+
if err != nil {
61+
return nil, 0, fmt.Errorf("error statting script file %q: %w", scriptPath, err)
62+
}
63+
perms := fileInfo.Mode().Perm()
64+
65+
decoded := make([]byte, base64.StdEncoding.DecodedLen(encoded.Len()))
66+
n, err := base64.StdEncoding.Decode(decoded, encoded.Bytes())
67+
if err != nil {
68+
return nil, 0, fmt.Errorf("error decoding script file %q: %w", scriptPath, err)
69+
}
70+
return decoded[0:n], perms, nil
71+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
Copyright 2020 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package subcommands
18+
19+
import (
20+
"encoding/base64"
21+
"errors"
22+
"io/ioutil"
23+
"os"
24+
"path/filepath"
25+
"testing"
26+
)
27+
28+
func TestDecodeScript(t *testing.T) {
29+
encoded := "IyEvdXNyL2Jpbi9lbnYgc2gKZWNobyAiSGVsbG8gV29ybGQhIgo="
30+
decoded := `#!/usr/bin/env sh
31+
echo "Hello World!"
32+
`
33+
mode := os.FileMode(0600)
34+
expectedPermissions := os.FileMode(0600)
35+
36+
tmp, err := ioutil.TempDir("", "decode-script-test-*")
37+
if err != nil {
38+
t.Fatalf("error creating temp file: %v", err)
39+
}
40+
src := filepath.Join(tmp, "script.txt")
41+
defer func() {
42+
if err := os.Remove(src); err != nil {
43+
t.Errorf("temporary script file %q was not cleaned up: %v", src, err)
44+
}
45+
}()
46+
if err = ioutil.WriteFile(src, []byte(encoded), mode); err != nil {
47+
t.Fatalf("error writing encoded script: %v", err)
48+
}
49+
50+
if err = decodeScript(src); err != nil {
51+
t.Errorf("unexpected error decoding script: %v", err)
52+
}
53+
54+
file, err := os.Open(src)
55+
if err != nil {
56+
t.Fatalf("unexpected error opening decoded script: %v", err)
57+
}
58+
defer file.Close()
59+
info, err := file.Stat()
60+
if err != nil {
61+
t.Fatalf("unexpected error statting decoded script: %v", err)
62+
}
63+
mod := info.Mode()
64+
b, err := ioutil.ReadAll(file)
65+
if err != nil {
66+
t.Fatalf("unexpected error reading content of decoded script: %v", err)
67+
}
68+
if string(b) != decoded {
69+
t.Errorf("expected decoded value %q received %q", decoded, string(b))
70+
}
71+
if mod != expectedPermissions {
72+
t.Errorf("expected mode %#o received %#o", expectedPermissions, mod)
73+
}
74+
}
75+
76+
func TestDecodeScriptMissingFileError(t *testing.T) {
77+
b, mod, err := decodeScriptFromFile("/path/to/non-existent/file")
78+
if !errors.Is(err, os.ErrNotExist) {
79+
t.Errorf("expected error %q received %q", os.ErrNotExist, err)
80+
}
81+
if b != nil || mod != 0 {
82+
t.Errorf("unexpected non-zero bytes or file mode returned")
83+
}
84+
}
85+
86+
func TestDecodeScriptInvalidBase64(t *testing.T) {
87+
invalidData := []byte("!")
88+
expectedError := base64.CorruptInputError(0)
89+
90+
src, err := ioutil.TempFile("", "decode-script-test-*")
91+
if err != nil {
92+
t.Fatalf("error creating temp file: %v", err)
93+
}
94+
defer func() {
95+
if err := os.Remove(src.Name()); err != nil {
96+
t.Errorf("temporary file %q was not cleaned up: %v", src.Name(), err)
97+
}
98+
}()
99+
if _, err := src.Write(invalidData); err != nil {
100+
t.Fatalf("error writing invalid base64 data: %v", err)
101+
}
102+
src.Close()
103+
104+
b, mod, err := decodeScriptFromFile(src.Name())
105+
if b != nil || mod != 0 {
106+
t.Errorf("unexpected non-zero bytes or file mode returned")
107+
}
108+
if !errors.Is(err, expectedError) {
109+
t.Errorf("expected error %q received %q", expectedError, err)
110+
}
111+
}

0 commit comments

Comments
 (0)