From e3401947a68c5f975d1c4d82de657d82b19fd8f8 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 May 2017 15:03:36 -0700 Subject: [PATCH] plugin/discovery: PluginRequirements can specify SHA256 digests As well as constraining plugins by version number, we also want to be able to pin plugins to use specific executables so that we can detect drift in available plugins between commands. This commit allows such requirements to be specified, but doesn't yet specify any such requirements, nor validate them. --- command/init.go | 2 +- moduledeps/module.go | 14 +++- moduledeps/module_test.go | 4 +- plugin/discovery/meta_set_test.go | 8 +-- plugin/discovery/requirements.go | 95 ++++++++++++++++++++++++--- plugin/discovery/requirements_test.go | 93 ++++++++++++++++++++++++++ plugin/discovery/version.go | 10 +++ plugin/discovery/version_set.go | 3 +- 8 files changed, 210 insertions(+), 19 deletions(-) create mode 100644 plugin/discovery/requirements_test.go diff --git a/command/init.go b/command/init.go index b3d816c31d2a..db6ec0fe2579 100644 --- a/command/init.go +++ b/command/init.go @@ -221,7 +221,7 @@ func (c *InitCommand) getProviders(path string, state *terraform.State) error { dst := c.pluginDir() for provider, reqd := range missing { - err := c.getProvider(dst, provider, reqd) + err := c.getProvider(dst, provider, reqd.Versions) // TODO: return all errors if err != nil { return err diff --git a/moduledeps/module.go b/moduledeps/module.go index 0e446108feff..d6cbaf5c5fd6 100644 --- a/moduledeps/module.go +++ b/moduledeps/module.go @@ -104,6 +104,9 @@ func (s sortModules) Swap(i, j int) { // This method only considers the direct requirements of the receiver. // Use AllPluginRequirements to flatten the dependencies for the // entire tree of modules. +// +// Requirements returned by this method include only version constraints, +// and apply no particular SHA256 hash constraint. func (m *Module) PluginRequirements() discovery.PluginRequirements { ret := make(discovery.PluginRequirements) for inst, dep := range m.Providers { @@ -111,12 +114,14 @@ func (m *Module) PluginRequirements() discovery.PluginRequirements { // a PluginRequirements wants keys to be provider *types*, such // as "aws". If there are multiple aliases for the same // provider then we will flatten them into a single requirement - // by using Intersection to merge the version sets. + // by combining their constraint sets. pty := inst.Type() if existing, exists := ret[pty]; exists { - ret[pty] = existing.Append(dep.Constraints) + ret[pty].Versions = existing.Versions.Append(dep.Constraints) } else { - ret[pty] = dep.Constraints + ret[pty] = &discovery.PluginConstraints{ + Versions: dep.Constraints, + } } } return ret @@ -125,6 +130,9 @@ func (m *Module) PluginRequirements() discovery.PluginRequirements { // AllPluginRequirements calls PluginRequirements for the receiver and all // of its descendents, and merges the result into a single PluginRequirements // structure that would satisfy all of the modules together. +// +// Requirements returned by this method include only version constraints, +// and apply no particular SHA256 hash constraint. func (m *Module) AllPluginRequirements() discovery.PluginRequirements { var ret discovery.PluginRequirements m.WalkTree(func(path []string, parent *Module, current *Module) error { diff --git a/moduledeps/module_test.go b/moduledeps/module_test.go index 630f3cd8b7f8..4cbe0103c8dd 100644 --- a/moduledeps/module_test.go +++ b/moduledeps/module_test.go @@ -207,10 +207,10 @@ func TestModulePluginRequirements(t *testing.T) { if len(reqd) != 2 { t.Errorf("wrong number of elements in %#v; want 2", reqd) } - if got, want := reqd["foo"].String(), ">=1.0.0,>=2.0.0"; got != want { + if got, want := reqd["foo"].Versions.String(), ">=1.0.0,>=2.0.0"; got != want { t.Errorf("wrong combination of versions for 'foo' %q; want %q", got, want) } - if got, want := reqd["baz"].String(), ">=3.0.0"; got != want { + if got, want := reqd["baz"].Versions.String(), ">=3.0.0"; got != want { t.Errorf("wrong combination of versions for 'baz' %q; want %q", got, want) } } diff --git a/plugin/discovery/meta_set_test.go b/plugin/discovery/meta_set_test.go index 18485f58d0f8..bd15c5be6d0c 100644 --- a/plugin/discovery/meta_set_test.go +++ b/plugin/discovery/meta_set_test.go @@ -310,10 +310,10 @@ func TestPluginMetaSetConstrainVersions(t *testing.T) { } byName := s.ConstrainVersions(PluginRequirements{ - "foo": ConstraintStr(">=2.0.0").MustParse(), - "bar": ConstraintStr(">=0.0.0").MustParse(), - "baz": ConstraintStr(">=1.0.0").MustParse(), - "fun": ConstraintStr(">5.0.0").MustParse(), + "foo": &PluginConstraints{Versions: ConstraintStr(">=2.0.0").MustParse()}, + "bar": &PluginConstraints{Versions: ConstraintStr(">=0.0.0").MustParse()}, + "baz": &PluginConstraints{Versions: ConstraintStr(">=1.0.0").MustParse()}, + "fun": &PluginConstraints{Versions: ConstraintStr(">5.0.0").MustParse()}, }) if got, want := len(byName), 3; got != want { t.Errorf("%d keys in map; want %d", got, want) diff --git a/plugin/discovery/requirements.go b/plugin/discovery/requirements.go index 1f58821e4538..75430fdd609f 100644 --- a/plugin/discovery/requirements.go +++ b/plugin/discovery/requirements.go @@ -1,26 +1,105 @@ package discovery +import ( + "bytes" +) + // PluginRequirements describes a set of plugins (assumed to be of a consistent // kind) that are required to exist and have versions within the given // corresponding sets. -// -// PluginRequirements is a map from plugin name to Constraints. -type PluginRequirements map[string]Constraints +type PluginRequirements map[string]*PluginConstraints + +// PluginConstraints represents an element of PluginRequirements describing +// the constraints for a single plugin. +type PluginConstraints struct { + // Specifies that the plugin's version must be within the given + // constraints. + Versions Constraints + + // If non-nil, the hash of the on-disk plugin executable must exactly + // match the SHA256 hash given here. + SHA256 []byte +} + +// Allows returns true if the given version is within the receiver's version +// constraints. +func (s *PluginConstraints) Allows(v Version) bool { + return s.Versions.Allows(v) +} + +// AcceptsSHA256 returns true if the given executable SHA256 hash is acceptable, +// either because it matches the constraint or because there is no such +// constraint. +func (s *PluginConstraints) AcceptsSHA256(digest []byte) bool { + if s.SHA256 == nil { + return true + } + return bytes.Equal(s.SHA256, digest) +} // Merge takes the contents of the receiver and the other given requirements // object and merges them together into a single requirements structure // that satisfies both sets of requirements. +// +// Note that it doesn't make sense to merge two PluginRequirements with +// differing required plugin SHA256 hashes, since the result will never +// match any plugin. func (r PluginRequirements) Merge(other PluginRequirements) PluginRequirements { ret := make(PluginRequirements) - for n, vs := range r { - ret[n] = vs + for n, c := range r { + ret[n] = &PluginConstraints{ + Versions: Constraints{}.Append(c.Versions), + SHA256: c.SHA256, + } } - for n, vs := range other { + for n, c := range other { if existing, exists := ret[n]; exists { - ret[n] = existing.Append(vs) + ret[n].Versions = ret[n].Versions.Append(c.Versions) + + if existing.SHA256 != nil { + if c.SHA256 != nil && !bytes.Equal(c.SHA256, existing.SHA256) { + // If we've been asked to merge two constraints with + // different SHA256 hashes then we'll produce a dummy value + // that can never match anything. This is a silly edge case + // that no reasonable caller should hit. + ret[n].SHA256 = []byte(invalidProviderHash) + } + } else { + ret[n].SHA256 = c.SHA256 // might still be nil + } } else { - ret[n] = vs + ret[n] = &PluginConstraints{ + Versions: Constraints{}.Append(c.Versions), + SHA256: c.SHA256, + } } } return ret } + +// LockExecutables applies additional constraints to the receiver that +// require plugin executables with specific SHA256 digests. This modifies +// the receiver in-place, since it's intended to be applied after +// version constraints have been resolved. +// +// The given map must include a key for every plugin that is already +// required. If not, any missing keys will cause the corresponding plugin +// to never match, though the direct caller doesn't necessarily need to +// guarantee this as long as the downstream code _applying_ these constraints +// is able to deal with the non-match in some way. +func (r PluginRequirements) LockExecutables(sha256s map[string][]byte) { + for name, cons := range r { + digest := sha256s[name] + + if digest == nil { + // Prevent any match, which will then presumably cause the + // downstream consumer of this requirements to report an error. + cons.SHA256 = []byte(invalidProviderHash) + continue + } + + cons.SHA256 = digest + } +} + +const invalidProviderHash = "" diff --git a/plugin/discovery/requirements_test.go b/plugin/discovery/requirements_test.go new file mode 100644 index 000000000000..b3300fab4bba --- /dev/null +++ b/plugin/discovery/requirements_test.go @@ -0,0 +1,93 @@ +package discovery + +import ( + "fmt" + "testing" +) + +func TestPluginConstraintsAllows(t *testing.T) { + tests := []struct { + Constraints *PluginConstraints + Version string + Want bool + }{ + { + &PluginConstraints{ + Versions: AllVersions, + }, + "1.0.0", + true, + }, + { + &PluginConstraints{ + Versions: ConstraintStr(">1.0.0").MustParse(), + }, + "1.0.0", + false, + }, + // This is not an exhaustive test because the callees + // already have plentiful tests of their own. + } + + for i, test := range tests { + t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { + version := VersionStr(test.Version).MustParse() + got := test.Constraints.Allows(version) + if got != test.Want { + t.Logf("looking for %s in %#v", test.Version, test.Constraints) + t.Errorf("wrong result %#v; want %#v", got, test.Want) + } + }) + } +} + +func TestPluginConstraintsAcceptsSHA256(t *testing.T) { + mustUnhex := func(hex string) (ret []byte) { + _, err := fmt.Sscanf(hex, "%x", &ret) + if err != nil { + panic(err) + } + return ret + } + + tests := []struct { + Constraints *PluginConstraints + Digest []byte + Want bool + }{ + { + &PluginConstraints{ + Versions: AllVersions, + SHA256: mustUnhex("0123456789abcdef"), + }, + mustUnhex("0123456789abcdef"), + true, + }, + { + &PluginConstraints{ + Versions: AllVersions, + SHA256: mustUnhex("0123456789abcdef"), + }, + mustUnhex("f00dface"), + false, + }, + { + &PluginConstraints{ + Versions: AllVersions, + SHA256: nil, + }, + mustUnhex("f00dface"), + true, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { + got := test.Constraints.AcceptsSHA256(test.Digest) + if got != test.Want { + t.Logf("%#v.AcceptsSHA256(%#v)", test.Constraints, test.Digest) + t.Errorf("wrong result %#v; want %#v", got, test.Want) + } + }) + } +} diff --git a/plugin/discovery/version.go b/plugin/discovery/version.go index f2706bd5301a..50d55635e603 100644 --- a/plugin/discovery/version.go +++ b/plugin/discovery/version.go @@ -19,6 +19,16 @@ func (s VersionStr) Parse() (Version, error) { return Version{raw}, nil } +// MustParse transforms a VersionStr into a Version if it is +// syntactically valid. If it isn't then it panics. +func (s VersionStr) MustParse() Version { + ret, err := s.Parse() + if err != nil { + panic(err) + } + return ret +} + // Version represents a version number that has been parsed from // a semver string and known to be valid. type Version struct { diff --git a/plugin/discovery/version_set.go b/plugin/discovery/version_set.go index e1b0b1e2c111..3ce5796899e8 100644 --- a/plugin/discovery/version_set.go +++ b/plugin/discovery/version_set.go @@ -43,7 +43,8 @@ func init() { } } -// Allows returns true if the given version is in the receiving set. +// Allows returns true if the given version permitted by the receiving +// constraints set. func (s Constraints) Allows(v Version) bool { return s.raw.Check(v.raw) }