From a423807b9134272676b1530e2888586b3e59387f Mon Sep 17 00:00:00 2001 From: Andrew Meyer Date: Fri, 17 Sep 2021 11:02:28 -0700 Subject: [PATCH] fixup; edge cases --- .gitignore | 1 + bindings/resolver.go | 71 ++++++++++++++++++--------------------- bindings/resolver_test.go | 66 ++++++++++++++++++++++++++++-------- 3 files changed, 85 insertions(+), 53 deletions(-) diff --git a/.gitignore b/.gitignore index 4befed30..ec05de3b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .DS_Store .idea +coverage.out diff --git a/bindings/resolver.go b/bindings/resolver.go index 955e2303..1db37801 100644 --- a/bindings/resolver.go +++ b/bindings/resolver.go @@ -68,7 +68,7 @@ type Resolver struct { } // NewResolver returns a new service binding resolver. The location of the bindings is given by one of the following, in -// the given order of precedence: +// the following order of precedence: // // 1. SERVICE_BINDING_ROOT environment variable // 2. CNB_BINDINGS environment variable, if above is not set @@ -143,7 +143,7 @@ func loadBindings(bindingRoot string) ([]Binding, error) { binding, err = loadBinding(bindingRoot, file.Name()) } if err != nil { - return nil, err + return nil, errors.Wrapf(err, "reading binding '%s'", file.Name()) } bindings = append(bindings, binding) } @@ -162,6 +162,7 @@ func isLegacyBinding(bindingRoot string, name string) (bool, error) { return false, err } +// See: https://github.com/k8s-service-bindings/spec#workload-projection func loadBinding(bindingRoot string, name string) (Binding, error) { binding := Binding{ Name: name, @@ -169,24 +170,24 @@ func loadBinding(bindingRoot string, name string) (Binding, error) { Secret: map[string]SecretReader{}, } - typeBytes, err := ioutil.ReadFile(filepath.Join(binding.Path, "type")) + secrets, err := loadSecrets(filepath.Join(binding.Path)) if err != nil { return Binding{}, err } - binding.Type = string(typeBytes) - providerBytes, err := ioutil.ReadFile(filepath.Join(binding.Path, "provider")) - if err != nil && !os.IsNotExist(err) { - return Binding{}, err - } - if err == nil { - binding.Provider = string(providerBytes) + typ, ok := secrets["type"] + if !ok { + return Binding{}, errors.New("missing 'type'") } + binding.Type = typ.String() + delete(secrets, "type") - secrets, err := loadSecrets(filepath.Join(binding.Path), "type", "provider") - if err != nil { - return Binding{}, err + provider, ok := secrets["provider"] + if ok { + binding.Provider = provider.String() + delete(secrets, "provider") } + binding.Secret = secrets return binding, nil @@ -200,36 +201,41 @@ func loadLegacyBinding(bindingRoot string, name string) (Binding, error) { Secret: map[string]SecretReader{}, } - typeBytes, err := ioutil.ReadFile(filepath.Join(binding.Path, "metadata", "kind")) + metadata, err := loadSecrets(filepath.Join(binding.Path, "metadata")) if err != nil { return Binding{}, err } - binding.Type = string(typeBytes) - providerBytes, err := ioutil.ReadFile(filepath.Join(binding.Path, "metadata", "provider")) - if err != nil { - return Binding{}, err + typ, ok := metadata["kind"] + if !ok { + return Binding{}, errors.New("missing 'kind'") } - binding.Provider = string(providerBytes) + binding.Type = typ.String() + delete(metadata, "kind") - metadata, err := loadSecrets(filepath.Join(binding.Path, "metadata"), "kind", "provider") - if err != nil { - return Binding{}, err + provider, ok := metadata["provider"] + if !ok { + return Binding{}, errors.New("missing 'provider'") } + binding.Provider = provider.String() + delete(metadata, "provider") + binding.Secret = metadata secrets, err := loadSecrets(filepath.Join(binding.Path, "secret")) - if err != nil { + if err != nil && !os.IsNotExist(err) { return Binding{}, err } - for k, v := range secrets { - binding.Secret[k] = v + if err == nil { + for k, v := range secrets { + binding.Secret[k] = v + } } return binding, nil } -func loadSecrets(secretsPath string, exclude ...string) (map[string]SecretReader, error) { +func loadSecrets(secretsPath string) (map[string]SecretReader, error) { secrets := map[string]SecretReader{} files, err := ioutil.ReadDir(secretsPath) if err != nil { @@ -237,10 +243,6 @@ func loadSecrets(secretsPath string, exclude ...string) (map[string]SecretReader } for _, file := range files { - if in(file.Name(), exclude) { - continue - } - content, err := os.ReadFile(filepath.Join(secretsPath, file.Name())) if err != nil { return nil, err @@ -249,12 +251,3 @@ func loadSecrets(secretsPath string, exclude ...string) (map[string]SecretReader } return secrets, nil } - -func in(item string, list []string) bool { - for _, s := range list { - if s == item { - return true - } - } - return false -} diff --git a/bindings/resolver_test.go b/bindings/resolver_test.go index 2517d0df..b6864bc9 100644 --- a/bindings/resolver_test.go +++ b/bindings/resolver_test.go @@ -283,7 +283,36 @@ func testResolver(t *testing.T, context spec.G, it spec.S) { )) }) - it("returns errors encountered while reading binding", func() { + it("returns an error if type is missing", func() { + err := os.MkdirAll(filepath.Join(bindingRoot, "bad-binding"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + _, err = resolver.Resolve("bad-type", "") + Expect(err).To(MatchError(HavePrefix("loading bindings from '%s': reading binding 'bad-binding': missing 'type'", bindingRoot))) + }) + + it("allows provider to be omitted", func() { + err := os.MkdirAll(filepath.Join(bindingRoot, "some-binding"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + err = os.WriteFile(filepath.Join(bindingRoot, "some-binding", "type"), []byte("some-type"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + binds, err := resolver.Resolve("some-type", "") + Expect(err).NotTo(HaveOccurred()) + + Expect(binds).To(ConsistOf( + bindings.Binding{ + Name: "some-binding", + Path: filepath.Join(bindingRoot, "some-binding"), + Type: "some-type", + Provider: "", + Secret: map[string]bindings.SecretReader{}, + }, + )) + }) + + it("returns errors encountered reading files", func() { err := os.MkdirAll(filepath.Join(bindingRoot, "bad-binding"), os.ModePerm) Expect(err).NotTo(HaveOccurred()) @@ -291,7 +320,7 @@ func testResolver(t *testing.T, context spec.G, it spec.S) { Expect(err).NotTo(HaveOccurred()) _, err = resolver.Resolve("bad-type", "") - Expect(err).To(MatchError(HavePrefix("loading bindings from '%s': open %s: permission denied", bindingRoot, filepath.Join(bindingRoot, "bad-binding", "type")))) + Expect(err).To(MatchError(HavePrefix("loading bindings from '%s': reading binding 'bad-binding': open %s: permission denied", bindingRoot, filepath.Join(bindingRoot, "bad-binding", "type")))) }) }) @@ -359,23 +388,17 @@ func testResolver(t *testing.T, context spec.G, it spec.S) { )) }) - it("allows 'kind' and 'provider' under secrets", func() { + it("allows 'secret' directory to be omitted", func() { err := os.MkdirAll(filepath.Join(bindingRoot, "binding-legacy", "metadata"), os.ModePerm) Expect(err).NotTo(HaveOccurred()) - err = os.MkdirAll(filepath.Join(bindingRoot, "binding-legacy", "secret"), os.ModePerm) - Expect(err).NotTo(HaveOccurred()) - err = os.WriteFile(filepath.Join(bindingRoot, "binding-legacy", "metadata", "kind"), []byte("type-legacy"), os.ModePerm) Expect(err).NotTo(HaveOccurred()) err = os.WriteFile(filepath.Join(bindingRoot, "binding-legacy", "metadata", "provider"), []byte("provider-legacy"), os.ModePerm) Expect(err).NotTo(HaveOccurred()) - err = os.WriteFile(filepath.Join(bindingRoot, "binding-legacy", "secret", "kind"), []byte("some-kind"), os.ModePerm) - Expect(err).NotTo(HaveOccurred()) - - err = os.WriteFile(filepath.Join(bindingRoot, "binding-legacy", "secret", "provider"), []byte("some-provider"), os.ModePerm) + err = os.WriteFile(filepath.Join(bindingRoot, "binding-legacy", "metadata", "some-key"), []byte("some-value"), os.ModePerm) Expect(err).NotTo(HaveOccurred()) binds, err := resolver.Resolve("type-legacy", "") @@ -388,15 +411,30 @@ func testResolver(t *testing.T, context spec.G, it spec.S) { Type: "type-legacy", Provider: "provider-legacy", Secret: map[string]bindings.SecretReader{ - "kind": bindings.MakeSecretReader([]byte("some-kind")), - "provider": bindings.MakeSecretReader([]byte("some-provider")), + "some-key": bindings.MakeSecretReader([]byte("some-value")), }, }, )) }) - it("allows 'secret' directory to be omitted", func() { - // TODO + it("returns an error if kind is missing", func() { + err := os.MkdirAll(filepath.Join(bindingRoot, "bad-binding", "metadata"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + _, err = resolver.Resolve("bad-type", "") + Expect(err).To(MatchError(HavePrefix("loading bindings from '%s': reading binding 'bad-binding': missing 'kind'", bindingRoot))) + }) + + it("returns an error if provider is missing", func() { + err := os.MkdirAll(filepath.Join(bindingRoot, "bad-binding", "metadata"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + err = os.WriteFile(filepath.Join(bindingRoot, "bad-binding", "metadata", "kind"), []byte("bad-type"), os.ModePerm) + Expect(err).NotTo(HaveOccurred()) + + _, err = resolver.Resolve("bad-type", "") + Expect(err).To(MatchError(HavePrefix("loading bindings from '%s': reading binding 'bad-binding': missing 'provider'", bindingRoot))) + }) }) })