fixup; edge cases

paketo-buildpacks · Sep 17, 2021 · a423807 · a423807
1 parent dae6884
commit a423807
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 53 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,3 @@
 .DS_Store
 .idea
+coverage.out
diff --git 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,31 +162,32 @@ 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,
 		Path:   filepath.Join(bindingRoot, name),
 		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,47 +201,48 @@ 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 {
 		return nil, err
 	}
 
 	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
@@ -283,15 +283,44 @@ 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())
 
 				err = os.WriteFile(filepath.Join(bindingRoot, "bad-binding", "type"), []byte("bad-type"), 000)
 				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)))
+
 			})
 		})
 	})