More cleanup

* Refactors Detect * More consistent name and constants Signed-off-by: Emily Casey <[email protected]>
paketo-buildpacks · Nov 2, 2020 · 9cb8643 · 9cb8643
1 parent 8230ad4
commit 9cb8643
Show file tree

Hide file tree

Showing 11 changed files with 127 additions and 122 deletions.
diff --git a/README.md b/README.md
@@ -24,7 +24,7 @@ To learn about the conventional meaning of `SSL_CERT_DIR` and `SSL_CERT_FILE` en
 ## Bindings
 The buildpack optionally accepts the following bindings:
 
-### Type: `ca-certficates`
+### Type: `ca-certificates`
 |Key                   | Value   | Description
 |----------------------|---------|------------
 |`<certificate-name>` | `<ceritificate>` | CA certficate to trust. Should contain exactly one PEM encoded certificate.

diff --git a/cacerts/build.go b/cacerts/build.go
@@ -43,39 +43,39 @@ func (b Build) Build(context libcnb.BuildContext) (libcnb.BuildResult, error) {
 	b.Logger.Title(context.Buildpack)
 
 	var certPaths []string
-	var contributeHelper bool
+	var contributedHelper bool
 	for _, e := range context.Plan.Entries {
 		switch strings.ToLower(e.Name) {
-		case PlanEntryCACertificates:
+		case PlanEntryCACerts:
 			paths, err := pathsFromEntryMetadata(e.Metadata)
 			if err != nil {
 				return libcnb.BuildResult{}, fmt.Errorf("failed to decode CA certificate paths from plan entry:\n%w", err)
 			}
 			certPaths = append(certPaths, paths...)
-		case PlanEntryCACertHelper:
-			contributeHelper = true
+		case PlanEntryCACertsHelper:
+			if contributedHelper {
+				continue
+			}
+			h := libpak.NewHelperLayerContributor(
+				context.Buildpack,
+				&context.Plan,
+				ExecutableCACertsHelper,
+			)
+			h.Logger = b.Logger
+			result.Layers = append(result.Layers, h)
+			contributedHelper = true
 		default:
 			return libcnb.BuildResult{}, fmt.Errorf("received unexpected buildpack plan entry %q", e.Name)
 		}
 	}
 
 	if len(certPaths) > 0 {
 		sort.Strings(certPaths)
-		layer := NewTrustedCAs(certPaths)
+		layer := NewTrustedCACerts(certPaths)
 		layer.Logger = b.Logger
 		result.Layers = append(result.Layers, layer)
 	}
 
-	if contributeHelper {
-		h := libpak.NewHelperLayerContributor(
-			context.Buildpack,
-			&context.Plan,
-			"ca-cert-helper",
-		)
-		h.Logger = b.Logger
-		result.Layers = append(result.Layers, h)
-	}
-
 	return result, nil
 }
 

diff --git a/cacerts/build_test.go b/cacerts/build_test.go
@@ -55,7 +55,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 		it.Before(func() {
 			ctx.Plan.Entries = []libcnb.BuildpackPlanEntry{
 				{
-					Name: "ca-certificates",
+					Name: cacerts.PlanEntryCACerts,
 					Metadata: map[string]interface{}{
 						"paths": []interface{}{
 							"some/path/cert1.pem",
@@ -73,7 +73,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 		it("contributes a ca-certificates layers", func() {
 			Expect(len(result.Layers)).To(BeNumerically(">=", 1))
 			Expect(result.Layers[0].Name()).To(Equal("ca-certificates"))
-			contributor, ok := result.Layers[0].(*cacerts.TrustedCAs)
+			contributor, ok := result.Layers[0].(*cacerts.TrustedCACerts)
 			Expect(ok).To(BeTrue())
 			Expect(len(contributor.CertPaths)).To(Equal(3))
 			Expect(contributor.CertPaths).To(ConsistOf([]string{
@@ -90,7 +90,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 		it.Before(func() {
 			ctx.Plan.Entries = []libcnb.BuildpackPlanEntry{
 				{
-					Name: "ca-certificates",
+					Name: cacerts.PlanEntryCACerts,
 					Metadata: map[string]interface{}{
 						"paths": []interface{}{
 							"some/path/cert1.pem",
@@ -99,7 +99,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 					},
 				},
 				{
-					Name: "ca-certificates",
+					Name: cacerts.PlanEntryCACerts,
 					Metadata: map[string]interface{}{
 						"paths": []interface{}{
 							"some/path/cert2.pem",
@@ -115,7 +115,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 		it("contributes a single ca-certificates", func() {
 			Expect(len(result.Layers)).To(BeNumerically(">=", 1))
 			Expect(result.Layers[0].Name()).To(Equal("ca-certificates"))
-			contributor, ok := result.Layers[0].(*cacerts.TrustedCAs)
+			contributor, ok := result.Layers[0].(*cacerts.TrustedCACerts)
 			Expect(ok).To(BeTrue())
 			Expect(len(contributor.CertPaths)).To(Equal(3))
 			Expect(contributor.CertPaths).To(Equal([]string{
@@ -131,7 +131,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 
 		it.Before(func() {
 			ctx.Plan.Entries = []libcnb.BuildpackPlanEntry{
-				{Name: "ca-cert-helper"},
+				{Name: cacerts.PlanEntryCACertsHelper},
 			}
 			var err error
 			result, err = build.Build(ctx)
@@ -151,8 +151,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
 
 		it.Before(func() {
 			ctx.Plan.Entries = []libcnb.BuildpackPlanEntry{
-				{Name: "ca-cert-helper"},
-				{Name: "ca-cert-helper"},
+				{Name: cacerts.PlanEntryCACertsHelper},
+				{Name: cacerts.PlanEntryCACertsHelper},
 			}
 			var err error
 			result, err = build.Build(ctx)

diff --git a/cacerts/certs.go b/cacerts/certs.go
@@ -36,8 +36,9 @@ import (
 // Environment variables and defaults used by openssl to load trusted CA certificates
 // (see https://www.openssl.org/docs/man1.1.0/man3/SSL_CTX_set_default_verify_paths.html)
 const (
-	// EnvCAPath is the environment variable that can be used to set CApath (see
-	EnvCAPath string = "SSL_CERT_DIR" // EnvCAPath
+	// EnvCAPath is the environment variable that can be used to set CApath
+	EnvCAPath string = "SSL_CERT_DIR"
+	// EnvCAFile is the environment variable that can be used to set CAfile
 	EnvCAFile string = "SSL_CERT_FILE"
 
 	// DefaultCAFile provides the default CAfile on ubuntu

diff --git a/cacerts/detect.go b/cacerts/detect.go
@@ -23,57 +23,48 @@ import (
 type Detect struct{}
 
 const (
-	// PlanEntryCACertificates if present in the build plan indicates that certificates should be added to the
+	// PlanEntryCACerts if present in the build plan indicates that certificates should be added to the
 	// truststore at build time.
-	PlanEntryCACertificates = "ca-certificates"
-	// PlanEntryCACertHelper if present in the build plan indicates the the ca-cert-helper binary should be
+	PlanEntryCACerts = "ca-certificates"
+	// PlanEntryCACertsHelper if present in the build plan indicates the the ca-cert-helper binary should be
 	// contributed to the app image.
-	PlanEntryCACertHelper = "ca-cert-helper"
+	PlanEntryCACertsHelper = "ca-certificates-helper"
 )
 
 // Detect always passes and optionally provides ca-certificates. If there is a binding of
 // type "ca-certificates" Detect also requires ca-certificates and provides an array of certificate paths in the
 // plan entry metadata.
 func (Detect) Detect(context libcnb.DetectContext) (libcnb.DetectResult, error) {
-	paths := getsCertsFromBindings(context.Platform.Bindings)
-	if len(paths) > 0 {
-		return libcnb.DetectResult{Pass: true, Plans: []libcnb.BuildPlan{{
-			Provides: []libcnb.BuildPlanProvide{
-				{Name: PlanEntryCACertificates},
-				{Name: PlanEntryCACertHelper},
-			},
-			Requires: []libcnb.BuildPlanRequire{
-				{
-					Name: PlanEntryCACertificates,
-					Metadata: map[string]interface{}{
-						"paths": paths,
-					},
-				},
-				{Name: PlanEntryCACertHelper},
-			},
-		}}}, nil
-	}
-
-	return libcnb.DetectResult{
+	result := libcnb.DetectResult{
 		Pass: true,
 		Plans: []libcnb.BuildPlan{
 			{
 				Provides: []libcnb.BuildPlanProvide{
-					{Name: PlanEntryCACertificates},
-					{Name: PlanEntryCACertHelper},
+					{Name: PlanEntryCACertsHelper},
+					{Name: PlanEntryCACerts},
 				},
 				Requires: []libcnb.BuildPlanRequire{
-					{Name: PlanEntryCACertHelper},
+					{Name: PlanEntryCACertsHelper},
 				},
 			},
 			{
 				Provides: []libcnb.BuildPlanProvide{
-					{Name: PlanEntryCACertHelper},
+					{Name: PlanEntryCACertsHelper},
 				},
 				Requires: []libcnb.BuildPlanRequire{
-					{Name: PlanEntryCACertHelper},
+					{Name: PlanEntryCACertsHelper},
 				},
 			},
 		},
-	}, nil
+	}
+	paths := getsCertsFromBindings(context.Platform.Bindings)
+	if len(paths) > 0 {
+		result.Plans[0].Requires = append(result.Plans[0].Requires, libcnb.BuildPlanRequire{
+			Name: PlanEntryCACerts,
+			Metadata: map[string]interface{}{
+				"paths": paths,
+			},
+		})
+	}
+	return result, nil
 }
diff --git a/cacerts/detect_test.go b/cacerts/detect_test.go
@@ -37,58 +37,54 @@ func testDetect(t *testing.T, context spec.G, it spec.S) {
 
 	it.Before(func() {
 		ctx.Platform.Environment = map[string]string{}
-		ctx.Buildpack.Metadata = map[string]interface{}{
-			"configurations": []map[string]interface{}{
-				{
-					"name":    "BP_RUNTIME_CACERTS_ENABLED",
-					"default": "true",
-				},
-			},
-		}
 	})
 
 	context("Binding exists with type ca-certificates", func() {
+		var result libcnb.DetectResult
 		it.Before(func() {
 			ctx.Platform.Bindings = []libcnb.Binding{
 				{
-					Type: "ca-certificates",
+					Type: cacerts.BindingType,
 					Path: "some-path",
 					Secret: map[string]string{
 						"cert1.pem": "",
 						"cert2.pem": "",
 					},
 				},
 				{
-					Type: "ca-certificates",
+					Type: cacerts.BindingType,
 					Path: "other-path",
 					Secret: map[string]string{
 						"cert3.pem": "",
 					},
 				},
 			}
+			var err error
+			result, err = detect.Detect(ctx)
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		it("always passes", func() {
+			Expect(result.Pass).To(BeTrue())
 		})
 
-		it("provides and requires ca-certificates and ca-cert-helper", func() {
-			Expect(detect.Detect(ctx)).To(Equal(libcnb.DetectResult{
-				Pass: true,
-				Plans: []libcnb.BuildPlan{
+		it("first plan provides and requires ca-certificate", func() {
+			Expect(len(result.Plans)).To(BeNumerically(">=", 1))
+			Expect(result.Plans[0]).To(Equal(libcnb.BuildPlan{
+				Provides: []libcnb.BuildPlanProvide{
+					{Name: cacerts.PlanEntryCACertsHelper},
+					{Name: cacerts.PlanEntryCACerts},
+				},
+				Requires: []libcnb.BuildPlanRequire{
+					{Name: cacerts.PlanEntryCACertsHelper},
 					{
-						Provides: []libcnb.BuildPlanProvide{
-							{Name: "ca-certificates"},
-							{Name: "ca-cert-helper"},
-						},
-						Requires: []libcnb.BuildPlanRequire{
-							{
-								Name: "ca-certificates",
-								Metadata: map[string]interface{}{
-									"paths": []string{
-										filepath.Join("other-path", "cert3.pem"),
-										filepath.Join("some-path", "cert1.pem"),
-										filepath.Join("some-path", "cert2.pem"),
-									},
-								},
+						Name: cacerts.PlanEntryCACerts,
+						Metadata: map[string]interface{}{
+							"paths": []string{
+								filepath.Join("other-path", "cert3.pem"),
+								filepath.Join("some-path", "cert1.pem"),
+								filepath.Join("some-path", "cert2.pem"),
 							},
-							{Name: "ca-cert-helper"},
 						},
 					},
 				},
@@ -97,27 +93,38 @@ func testDetect(t *testing.T, context spec.G, it spec.S) {
 	})
 
 	context("Binding does not exist with type ca-certificates", func() {
-		it("provides ca-certificates and provides and requires ca-cert-helper", func() {
-			Expect(detect.Detect(ctx)).To(Equal(libcnb.DetectResult{
-				Pass: true,
-				Plans: []libcnb.BuildPlan{
-					{
-						Provides: []libcnb.BuildPlanProvide{
-							{Name: "ca-certificates"},
-							{Name: "ca-cert-helper"},
-						},
-						Requires: []libcnb.BuildPlanRequire{
-							{Name: "ca-cert-helper"},
-						},
-					},
-					{
-						Provides: []libcnb.BuildPlanProvide{
-							{Name: "ca-cert-helper"},
-						},
-						Requires: []libcnb.BuildPlanRequire{
-							{Name: "ca-cert-helper"},
-						},
-					},
+		var result libcnb.DetectResult
+		it.Before(func() {
+			var err error
+			result, err = detect.Detect(ctx)
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		it("always passes", func() {
+			Expect(result.Pass).To(BeTrue())
+		})
+
+		it("first plan provides ca-certificates", func() {
+			Expect(len(result.Plans)).To(BeNumerically(">=", 1))
+			Expect(result.Plans[0]).To(Equal(libcnb.BuildPlan{
+				Provides: []libcnb.BuildPlanProvide{
+					{Name: cacerts.PlanEntryCACertsHelper},
+					{Name: cacerts.PlanEntryCACerts},
+				},
+				Requires: []libcnb.BuildPlanRequire{
+					{Name: cacerts.PlanEntryCACertsHelper},
+				},
+			}))
+		})
+
+		it("second plan always contributes the ca-certs-helper", func() {
+			Expect(len(result.Plans)).To(Equal(2))
+			Expect(result.Plans[1]).To(Equal(libcnb.BuildPlan{
+				Provides: []libcnb.BuildPlanProvide{
+					{Name: cacerts.PlanEntryCACertsHelper},
+				},
+				Requires: []libcnb.BuildPlanRequire{
+					{Name: cacerts.PlanEntryCACertsHelper},
 				},
 			}))
 		})

diff --git a/cacerts/execd.go b/cacerts/execd.go
@@ -28,6 +28,12 @@ import (
 	"github.com/paketo-buildpacks/libpak/bard"
 )
 
+const (
+	// ExecutableCACertsHelper provides the name of the exec.d executable that adds CA certificates to the truststore
+	// at runtime.
+	ExecutableCACertsHelper = "ca-certificates-helper"
+)
+
 type ExecD struct {
 	Logger            bard.Logger
 	Bindings          libcnb.Bindings
@@ -50,12 +56,12 @@ func (e *ExecD) Execute() (map[string]string, error) {
 	if len(paths) == 0 {
 		return env, nil
 	}
-	certDir, err := ioutil.TempDir("", "ca-certs")
+	certDir, err := ioutil.TempDir("", "ca-certificates")
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temp dir\n%w", err)
 	}
 	if err := e.GenerateHashLinks(certDir, paths); err != nil {
-		return nil, fmt.Errorf("failed to generate ca certficate symlinks\n%w", err)
+		return nil, fmt.Errorf("failed to generate CA certficate symlinks\n%w", err)
 	}
 	e.Logger.Infof("Added %d additional CA certificate(s) to system truststore", len(paths))
 

diff --git a/cacerts/init_test.go b/cacerts/init_test.go
@@ -29,6 +29,6 @@ func TestUnit(t *testing.T) {
 	suite("Build", testBuild)
 	suite("ExecD", testExecD)
 	suite("Certs", testCerts)
-	suite("TrustedCAs", testLayer)
+	suite("TrustedCACerts", testTrustedCACerts)
 	suite.Run(t)
 }