Skip to content

Commit

Permalink
Encode scripts as base 64 to avoid k8s mangling "$$"
Browse files Browse the repository at this point in the history
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](tektoncd#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.
  • Loading branch information
Scott authored and tekton-robot committed Jun 14, 2021
1 parent ccf723a commit 584b527
Show file tree
Hide file tree
Showing 13 changed files with 642 additions and 101 deletions.
38 changes: 8 additions & 30 deletions cmd/entrypoint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ package main

import (
"flag"
"io"
"log"
"os"
"os/exec"
"strings"
"syscall"
"time"

"github.com/tektoncd/pipeline/cmd/entrypoint/subcommands"
"github.com/tektoncd/pipeline/pkg/credentials"
"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
Expand All @@ -45,25 +45,6 @@ var (

const defaultWaitPollingInterval = time.Second

func cp(src, dst string) error {
s, err := os.Open(src)
if err != nil {
return err
}
defer s.Close()

// Owner has permission to write and execute, and anybody has
// permission to execute.
d, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE, 0311)
if err != nil {
return err
}
defer d.Close()

_, err = io.Copy(d, s)
return err
}

func main() {
// Add credential flags originally introduced with our legacy credentials helper
// image (creds-init).
Expand All @@ -72,17 +53,14 @@ func main() {

flag.Parse()

// If invoked in "cp mode" (`entrypoint cp <src> <dst>`), simply copy
// the src path to the dst path. This is used to place the entrypoint
// binary in the tools directory, without requiring the cp command to
// exist in the base image.
if len(flag.Args()) == 3 && flag.Args()[0] == "cp" {
src, dst := flag.Args()[1], flag.Args()[2]
if err := cp(src, dst); err != nil {
log.Fatal(err)
if err := subcommands.Process(flag.Args()); err != nil {
log.Println(err.Error())
switch err.(type) {
case subcommands.SubcommandSuccessful:
return
default:
os.Exit(1)
}
log.Println("Copied", src, "to", dst)
return
}

// Copy credentials we're expecting from the legacy credentials helper (creds-init)
Expand Down
46 changes: 46 additions & 0 deletions cmd/entrypoint/subcommands/cp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
Copyright 2020 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 subcommands

import (
"io"
"os"
)

const CopyCommand = "cp"

// Owner has permission to write and execute, and anybody has
// permission to execute.
const dstPermissions = 0311

// cp copies a files from src to dst.
func cp(src, dst string) error {
s, err := os.Open(src)
if err != nil {
return err
}
defer s.Close()

d, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE, dstPermissions)
if err != nil {
return err
}
defer d.Close()

_, err = io.Copy(d, s)
return err
}
69 changes: 69 additions & 0 deletions cmd/entrypoint/subcommands/cp_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
Copyright 2020 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 subcommands

import (
"errors"
"io/ioutil"
"os"
"path/filepath"
"testing"
)

func TestCp(t *testing.T) {
tmp, err := ioutil.TempDir("", "cp-test-*")
if err != nil {
t.Fatalf("error creating temp directory: %v", err)
}
defer os.RemoveAll(tmp)
src := filepath.Join(tmp, "foo.txt")
dst := filepath.Join(tmp, "bar.txt")

if err = ioutil.WriteFile(src, []byte("hello world"), 0700); err != nil {
t.Fatalf("error writing source file: %v", err)
}

if err := cp(src, dst); err != nil {
t.Errorf("error copying: %v", err)
}

info, err := os.Lstat(dst)
if err != nil {
t.Fatalf("error statting destination file: %v", err)
}

if info.Mode().Perm() != dstPermissions {
t.Errorf("expected permissions %#o for destination file but found %#o", dstPermissions, info.Mode().Perm())
}
}

func TestCpMissingFile(t *testing.T) {
tmp, err := ioutil.TempDir("", "cp-test-*")
if err != nil {
t.Fatalf("error creating temp directory: %v", err)
}
defer os.RemoveAll(tmp)
src := filepath.Join(tmp, "doesnt-exist.txt")
dst := filepath.Join(tmp, "bar.txt")
err = cp(src, dst)
if err == nil {
t.Errorf("unexpected success copying missing file")
}
if !errors.Is(err, os.ErrNotExist) {
t.Errorf(`expected "file does not exist" error but received %v`, err)
}
}
71 changes: 71 additions & 0 deletions cmd/entrypoint/subcommands/decode_script.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright 2020 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 subcommands

import (
"bytes"
"encoding/base64"
"fmt"
"io"
"io/ioutil"
"os"
)

const DecodeScriptCommand = "decode-script"

// decodeScript rewrites a script file from base64 back into its original content from
// the Step definition.
func decodeScript(scriptPath string) error {
decodedBytes, permissions, err := decodeScriptFromFile(scriptPath)
if err != nil {
return fmt.Errorf("error decoding script file %q: %w", scriptPath, err)
}
err = ioutil.WriteFile(scriptPath, decodedBytes, permissions)
if err != nil {
return fmt.Errorf("error writing decoded script file %q: %w", scriptPath, err)
}
return nil
}

// decodeScriptFromFile reads the script at scriptPath, decodes it from
// base64, and returns the decoded bytes w/ the permissions to use when re-writing
// or an error.
func decodeScriptFromFile(scriptPath string) ([]byte, os.FileMode, error) {
scriptFile, err := os.Open(scriptPath)
if err != nil {
return nil, 0, fmt.Errorf("error reading from script file %q: %w", scriptPath, err)
}
defer scriptFile.Close()

encoded := bytes.NewBuffer(nil)
if _, err = io.Copy(encoded, scriptFile); err != nil {
return nil, 0, fmt.Errorf("error reading from script file %q: %w", scriptPath, err)
}

fileInfo, err := scriptFile.Stat()
if err != nil {
return nil, 0, fmt.Errorf("error statting script file %q: %w", scriptPath, err)
}
perms := fileInfo.Mode().Perm()

decoded := make([]byte, base64.StdEncoding.DecodedLen(encoded.Len()))
n, err := base64.StdEncoding.Decode(decoded, encoded.Bytes())
if err != nil {
return nil, 0, fmt.Errorf("error decoding script file %q: %w", scriptPath, err)
}
return decoded[0:n], perms, nil
}
111 changes: 111 additions & 0 deletions cmd/entrypoint/subcommands/decode_script_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2020 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 subcommands

import (
"encoding/base64"
"errors"
"io/ioutil"
"os"
"path/filepath"
"testing"
)

func TestDecodeScript(t *testing.T) {
encoded := "IyEvdXNyL2Jpbi9lbnYgc2gKZWNobyAiSGVsbG8gV29ybGQhIgo="
decoded := `#!/usr/bin/env sh
echo "Hello World!"
`
mode := os.FileMode(0600)
expectedPermissions := os.FileMode(0600)

tmp, err := ioutil.TempDir("", "decode-script-test-*")
if err != nil {
t.Fatalf("error creating temp file: %v", err)
}
src := filepath.Join(tmp, "script.txt")
defer func() {
if err := os.Remove(src); err != nil {
t.Errorf("temporary script file %q was not cleaned up: %v", src, err)
}
}()
if err = ioutil.WriteFile(src, []byte(encoded), mode); err != nil {
t.Fatalf("error writing encoded script: %v", err)
}

if err = decodeScript(src); err != nil {
t.Errorf("unexpected error decoding script: %v", err)
}

file, err := os.Open(src)
if err != nil {
t.Fatalf("unexpected error opening decoded script: %v", err)
}
defer file.Close()
info, err := file.Stat()
if err != nil {
t.Fatalf("unexpected error statting decoded script: %v", err)
}
mod := info.Mode()
b, err := ioutil.ReadAll(file)
if err != nil {
t.Fatalf("unexpected error reading content of decoded script: %v", err)
}
if string(b) != decoded {
t.Errorf("expected decoded value %q received %q", decoded, string(b))
}
if mod != expectedPermissions {
t.Errorf("expected mode %#o received %#o", expectedPermissions, mod)
}
}

func TestDecodeScriptMissingFileError(t *testing.T) {
b, mod, err := decodeScriptFromFile("/path/to/non-existent/file")
if !errors.Is(err, os.ErrNotExist) {
t.Errorf("expected error %q received %q", os.ErrNotExist, err)
}
if b != nil || mod != 0 {
t.Errorf("unexpected non-zero bytes or file mode returned")
}
}

func TestDecodeScriptInvalidBase64(t *testing.T) {
invalidData := []byte("!")
expectedError := base64.CorruptInputError(0)

src, err := ioutil.TempFile("", "decode-script-test-*")
if err != nil {
t.Fatalf("error creating temp file: %v", err)
}
defer func() {
if err := os.Remove(src.Name()); err != nil {
t.Errorf("temporary file %q was not cleaned up: %v", src.Name(), err)
}
}()
if _, err := src.Write(invalidData); err != nil {
t.Fatalf("error writing invalid base64 data: %v", err)
}
src.Close()

b, mod, err := decodeScriptFromFile(src.Name())
if b != nil || mod != 0 {
t.Errorf("unexpected non-zero bytes or file mode returned")
}
if !errors.Is(err, expectedError) {
t.Errorf("expected error %q received %q", expectedError, err)
}
}
Loading

0 comments on commit 584b527

Please sign in to comment.