Ginkgo 2.0 introduced [new features](https://onsi.github.io/ginkgo/MIGRATING_TO_V2#major-additions-and-improvement)
that substatially improve developer experience. It is also now the only
actively developed and supported version of Ginkgo.
Co-authored-by: Rui Yang <ruiya@vmware.com>
Signed-off-by: David Timm <dtimm@vmware.com>
by default, "encoding/json" (and hence "sigs.k8s.io/yaml") decode any
number as float64 (if you try to decode into a generic type like
interface{})
vars/template.go does not allow interpolating float64 into a string,
presumably because you could end up interpolating the number 123.0 as
e.g. 122.99999999999999 due to limitations with float precision.
this means that if you ever load_var on a file containing a number
(even an integer), then you can never interpolate that number into a
string, which can be pretty annoying.
json.Number, on the other hand, stores the number as a string, so there
is no issue with precision - and it means ints will stay as "ints".
since we never use these values directly (they are just
serialized/deserialized), this *shouldn't* cause issues.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
we quoted / when it appeared in an instance var's value, but not when it
appeared in the key. while we still parse var references properly if
they contain a / without quoting, when we generate the string, we should
quote it for consistency
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
Co-authored-by: Esteban Foronda <eforonda@vmware.com>
Apologies for this unfocused commit - it does several things:
* Rename WebQueryParams and InstanceVarsFromWebQueryParams by removing
the Web specification
* Remove the old instance_vars implementation of API query params
* Note that this breaks a ton of code/tests - going to hold this off
for subsequent commits
* Change the query param prefix from var to vars, which felt more
natural
* This will of course require a change in #6105 as well
* Allow specifying the root-level vars in full. This makes certain APIs
more convenient, and this ends up making the query param API a
generalization of the old instance_vars one
* Make parsing query params strict (i.e. don't ignore errors)
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
A few issues:
- We didn't quote special characters (e.g. comma), which would result in
the flag not working if you copy/paste it directly into fly
- We marshaled everything as YAML, which has unpleasant results for
lists
- We weren't quoting fields with spaces in them
This switches to JSON marshaling, but explicitly unquotes values that
don't require quoting (i.e. don't have special characters and don't
change type when unmarshaled as YAML unquoted).
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
concourse/concourse#6041
...and use the references to properly construct a StaticVariables.
This also changes the YAMLVariableFlag slightly - rather than using
"gopkg.in/yaml.v2" to parse the YAML value, it uses "sigs.k8s.io/yaml",
which converts YAML to JSON first, and then unmarshals the JSON. This is
the package we use in most of Concourse. The reason for the change is
that parsing the YAML directly results in YAML objects being unmarshaled
as `map[interface{}]interface{}`, which is less than ideal. However, it
also means that numbers will be interpolated as `float64`, which is also
less than ideal - so, instead, we unmarshal numbers as `json.Number`,
which is just the string value of the number that's passed in.
We could possibly get rid of the map[interface{}]interface{} handling in
the vars package now, but I'm a bit worried about the implications of
that, so I'll hold off for now.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
I noticed a subtle bug in var parsing:
* ((vault:"my:var")) parses correctly, but
* (("my:var")) does not (the Source is set as `"my`, and the path as
`var`)
This commit fixes that bug by manually parsing the reference rather than
using a regular expression. It's easier to have fine control over the
parsing this way IMO.
I also made vars.ParseReference public - I was planning on doing so,
anyway, as I intend to use it outside of vars.Template (specifically,
for parsing variable flags)
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
concourse/concourse#6041
Adds a new KVPairs type that is a list of key-value pairs. KVPairs can
be Expanded into a StaticVariables (which supports the usual Get/List
methods of a Variables impl), and StaticVariables can be Flattened into
a KVPairs.
The intention of Expand is to go from a list of var flags to a
StaticVariables for interpolating the pipeline template. The intention
of Flatten is to convert a pipeline's instance vars to a flat string
form using dot notation.
Note that we already have an implementation of both methods in
atc/pipeline.go for instance vars - however, I intend to use this
simplified implementation that does not traverse lists since the
notation for doing so is ambiguous (if a field name is an integer, it
assumes that it's an index in a list). Instead of using dot notation for
lists, we'll just display the list as '[a, b, c]'
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
...rather than tracking the raw name that's passed into the template. It
felt a bit odd to me that we tracked this name, given that we never
really used it (practically only for error messages). It was also a bit
worrisome that we only sometimes set the Name, which felt like a
potential bug waiting to happen if we ever decided to use the Name for
something.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
Previously, vars.Variable implementations only considered the `Path`
(and occasionally, `Source`) fields, and completely ignored the `Fields`
of the passed in `vars.Reference` - it was up to `vars.Template` to
actually traverse the fields.
This is certainly odd - I would expect as a caller of a `vars.Variables`
that the Fields would be traversed given that I'm passing in a full
Reference.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
I'm not sure if the Type/Option fields were ever used, but they aren't
anymore - so, let's just get rid of the wrapper type and use
vars.Reference instead
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
It's already namespaced to the `vars` package, so adding `Variable` is
redundant. Note that I didn't do this for `VariableDefinition`, since
it's getting the boot
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
JSON-escaped quotes were not being removed from subkeys and secret
fields that are quoted because they contain special characters.
Signed-off-by: Anshul Sirur <vixus0@gmail.com>
Something specific to Builds didn't belong outside of the atc package.
Given that it's only consumed directly by the exec.RunState, it made
sense to go there.
I considered just inlining all of the logic in RunState, but decided
it's better to keep RunState lean, since it already has a few
responsibilities.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
I realized that local scope wasn't being handled properly before. We
created a clone of the local vars when we called `NewLocalScope`, but
local vars are typically populated at runtime - not in the
`engine/builder`. So, the clone was always empty. Instead, form a linked
list of `BuildVariables` that point to its parent scope.
This commit also renames `CredVarsTracker` to `BuildVariables`, and
exposes the struct directly rather than an interface. This commit is
incomplete in the sense that it doesn't perform that renaming
everywhere, but the diff got pretty huge and I wanted to separate out
just the behaviour aspect of this change.
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
In order for the across step to function properly, each instance of a
step run `across` multiple values must have its own local scope. This
way, the each step will have its own version of the value its iterating
over, and any `get_var` or `load_var` steps run in an `across` step
won't pollute the others (since `across` steps can run multiple things
in parallel).
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
* extract source, path and fields from secrect name instead of treating it
as black box
* move nested variables lookup out of template into static_vars
* refactor trackers error handling
Signed-off-by: Rui Yang <ryang@pivotal.io>
Co-authored-by: Bohan Chen <bochen@pivotal.io>
as we standardize on wrapping errors using `errors` (rather than
`github.com/pkg/errors), we can also make our testing standard (using
`errors.Is()` to check if we got a specific err in the chain).
Signed-off-by: Ciro S. Costa <cscosta@pivotal.io>
With this change, for all secrets fetched from credential manager, if they appear
in build logs, then they will be automatically redacted as "[**redacted**]]".
As this is currently an exprimental feature, a "concourse web" command option
"--enable-redact-secrets" has been added to turn on this feature.
Close#4311
Signed-off-by: Chao Li <chaol@vmware.com>
this is the fork of github.com/ghodss/yaml maintained by the k8s project
bumped a few dependencies necessary to get github.com/ghodss/yaml out of
the dependency graph; notably k8s and etcd (which is a transitive
dependency of the vault cred manager)
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Define '*Error' types instead of generating an error string each time,
and update the wording to be more consistent with the rest of the errors
(mainly, lowercase them)
side note: I noticed a few places where these error strings were faked
out in a HTTP response for tests, but those tests don't actually care
about the error message, so I changed them to just use a filler message.
Those tests should probably be refactored to assert on the only actual
change in behavior: that the ?check_creds query param is set.
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
this fixes auto-import conflicts with text/template and moves it
somewhere central so fly doesn't have to import it from the atc
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
David Timm