Skip to content

Commit

Permalink
fix(jsonnet): Restore tk.env (#482)
Browse files Browse the repository at this point in the history
* fix(jsonnet): Restore tk.env

In a recent refactor we lost `tk.env` respectivley
`std.extVar("tanka.dev/environment")` support.

For static environment this however is still the way to go for accessing
env data from within Jsonnet.

This PR restores the functionality and also makes Tanka properly fail
with a message when attempting to use `tk.env` from an inline environment

* fix(api): Prevent race condition

Because jsonnet.Opts includes a `map[string]string`, it is not
concurrency-safe, as all goroutines effectively share the same extCode
and tlaCode.

Modifications of any routine is also reflected onto every other routine.

To temporarily combat that, this PR deep-clones jsonnet.Opts before
passing it to any goroutine.

This is not a good permanent solution, because it can be easily
forgotten. We should instead look into ways of fixing the jsonnet
package so that such race conditions don't occur at all
  • Loading branch information
sh0rez authored Jan 21, 2021
1 parent 5d821cc commit 2caa673
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 0 deletions.
19 changes: 19 additions & 0 deletions pkg/jsonnet/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ type Opts struct {
EvalScript string
}

// Clone returns a deep copy of Opts
func (o Opts) Clone() Opts {
var extCode, tlaCode InjectedCode
for k, v := range o.ExtCode {
extCode[k] = v
}

for k, v := range o.TLACode {
tlaCode[k] = v
}

return Opts{
TLACode: tlaCode,
ExtCode: extCode,
ImportPaths: append([]string{}, o.ImportPaths...),
EvalScript: o.EvalScript,
}
}

// MakeVM returns a Jsonnet VM with some extensions of Tanka, including:
// - extended importer
// - extCode and tlaCode applied
Expand Down
3 changes: 3 additions & 0 deletions pkg/tanka/inline.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ func (i *InlineLoader) List(path string, opts LoaderOpts) ([]*v1alpha1.Environme
}

func inlineEval(path string, opts JsonnetOpts) (manifest.List, error) {
// Can't provide env as extVar, as we need to evaluate Jsonnet first to know it
opts.ExtCode.Set(environmentExtCode, `error "Using tk.env and std.extVar('tanka.dev/environment') is only supported for static environments. Directly access this data using standard Jsonnet instead."`)

raw, err := EvalJsonnet(path, opts)
if err != nil {
return nil, err
Expand Down
4 changes: 4 additions & 0 deletions pkg/tanka/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import (
"github.com/pkg/errors"
)

// environmentExtCode is the extCode ID `tk.env` uses underneath
// TODO: remove "import tk" and replace it with tanka-util
const environmentExtCode = spec.APIGroup + "/environment"

// Load loads the Environment at `path`. It automatically detects whether to
// load inline or statically
func Load(path string, opts Opts) (*LoadResult, error) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/tanka/parallel.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ func parallelLoadEnvironments(paths []string, opts parallelOpts) ([]*v1alpha1.En

for name, path := range list {
o := opts.Opts

// TODO: This is required because the map[string]string in here is not
// concurrency-safe. Instead of putting this burden on the caller, find
// a way to handle this inside the jsonnet package. A possible way would
// be to make the jsonnet package less general, more tightly coupling it
// to Tanka workflow thus being able to handle such cases
o.JsonnetOpts = o.JsonnetOpts.Clone()

o.Name = name
jobsCh <- parallelJob{
path: path,
Expand Down
16 changes: 16 additions & 0 deletions pkg/tanka/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ func (s StaticLoader) Load(path string, opts LoaderOpts) (*v1alpha1.Environment,
return nil, err
}

envCode, err := specToExtCode(*config)
if err != nil {
return nil, err
}
opts.ExtCode.Set(environmentExtCode, envCode)

data, err := EvalJsonnet(path, opts.JsonnetOpts)
if err != nil {
return nil, err
Expand Down Expand Up @@ -48,6 +54,16 @@ func (s StaticLoader) List(path string, opts LoaderOpts) ([]*v1alpha1.Environmen
return []*v1alpha1.Environment{env}, nil
}

func specToExtCode(spec v1alpha1.Environment) (string, error) {
spec.Data = nil
data, err := json.Marshal(spec)
if err != nil {
return "", err
}

return string(data), nil
}

// parseStaticSpec parses the `spec.json` of the environment and returns a
// *kubernetes.Kubernetes from it
func parseStaticSpec(path string) (*v1alpha1.Environment, error) {
Expand Down

0 comments on commit 2caa673

Please sign in to comment.