From 9cb86434b9b594efaa6defeb632bc1072f14eb93 Mon Sep 17 00:00:00 2001 From: Emily Casey Date: Mon, 2 Nov 2020 18:39:21 -0500 Subject: [PATCH] More cleanup * Refactors Detect * More consistent name and constants Signed-off-by: Emily Casey --- README.md | 2 +- cacerts/build.go | 30 ++--- cacerts/build_test.go | 16 +-- cacerts/certs.go | 5 +- cacerts/detect.go | 51 ++++----- cacerts/detect_test.go | 107 ++++++++++-------- cacerts/execd.go | 10 +- cacerts/init_test.go | 2 +- .../{trusted_cas.go => trusted_ca_certs.go} | 14 +-- ...d_cas_test.go => trusted_ca_certs_test.go} | 10 +- cmd/helper/main.go | 2 +- 11 files changed, 127 insertions(+), 122 deletions(-) rename cacerts/{trusted_cas.go => trusted_ca_certs.go} (83%) rename cacerts/{trusted_cas_test.go => trusted_ca_certs_test.go} (91%) diff --git a/README.md b/README.md index bfbd947..686647c 100644 --- 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 |----------------------|---------|------------ |`` | `` | CA certficate to trust. Should contain exactly one PEM encoded certificate. diff --git a/cacerts/build.go b/cacerts/build.go index 8b8208a..64535d9 100644 --- a/cacerts/build.go +++ b/cacerts/build.go @@ -43,17 +43,27 @@ 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) } @@ -61,21 +71,11 @@ func (b Build) Build(context libcnb.BuildContext) (libcnb.BuildResult, error) { 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 index 8705c58..14c5f06 100644 --- 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 index 63475bd..876538a 100644 --- 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 index 2d93fe8..bc878ba 100644 --- 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 index 44630fb..a0fb160 100644 --- a/cacerts/detect_test.go +++ b/cacerts/detect_test.go @@ -37,21 +37,14 @@ 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": "", @@ -59,36 +52,39 @@ func testDetect(t *testing.T, context spec.G, it spec.S) { }, }, { - 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 index 5ecd3f9..50d94bd 100644 --- 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 index 1e1e8de..5d2b70c 100644 --- 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) } diff --git a/cacerts/trusted_cas.go b/cacerts/trusted_ca_certs.go similarity index 83% rename from cacerts/trusted_cas.go rename to cacerts/trusted_ca_certs.go index 71e2e66..13cfd89 100644 --- a/cacerts/trusted_cas.go +++ b/cacerts/trusted_ca_certs.go @@ -27,15 +27,15 @@ import ( "github.com/paketo-buildpacks/libpak/bard" ) -type TrustedCAs struct { +type TrustedCACerts struct { CertPaths []string LayerContributor libpak.LayerContributor GenerateHashLinks func(dir string, certPaths []string) error Logger bard.Logger } -func NewTrustedCAs(paths []string) *TrustedCAs { - return &TrustedCAs{ +func NewTrustedCACerts(paths []string) *TrustedCACerts { + return &TrustedCACerts{ CertPaths: paths, LayerContributor: libpak.NewLayerContributor("CA Certificates", map[string]interface{}{}), GenerateHashLinks: GenerateHashLinks, @@ -43,18 +43,18 @@ func NewTrustedCAs(paths []string) *TrustedCAs { } // Contribute create build layer adding the certificates at Layer.CAPaths to the set of trusted CAs. -func (l TrustedCAs) Contribute(layer libcnb.Layer) (libcnb.Layer, error) { +func (l TrustedCACerts) Contribute(layer libcnb.Layer) (libcnb.Layer, error) { l.LayerContributor.Logger = l.Logger return l.LayerContributor.Contribute(layer, func() (libcnb.Layer, error) { layer.BuildEnvironment = libcnb.Environment{} - certsDir := filepath.Join(layer.Path, "certs") + certsDir := filepath.Join(layer.Path, "ca-certificates") if err := os.Mkdir(certsDir, 0777); err != nil { return libcnb.Layer{}, fmt.Errorf("failed to create directory %q\n%w", certsDir, err) } if err := l.GenerateHashLinks(certsDir, l.CertPaths); err != nil { - return libcnb.Layer{}, fmt.Errorf("failed to generate certificate symlinks\n%w", err) + return libcnb.Layer{}, fmt.Errorf("failed to generate CA certificate symlinks\n%w", err) } l.Logger.Bodyf("Added %d additional CA certificate(s) to system truststore", len(l.CertPaths)) @@ -68,6 +68,6 @@ func (l TrustedCAs) Contribute(layer libcnb.Layer) (libcnb.Layer, error) { }, libpak.BuildLayer) } -func (TrustedCAs) Name() string { +func (TrustedCACerts) Name() string { return "ca-certificates" } diff --git a/cacerts/trusted_cas_test.go b/cacerts/trusted_ca_certs_test.go similarity index 91% rename from cacerts/trusted_cas_test.go rename to cacerts/trusted_ca_certs_test.go index 54a5087..12367be 100644 --- a/cacerts/trusted_cas_test.go +++ b/cacerts/trusted_ca_certs_test.go @@ -30,11 +30,11 @@ import ( "github.com/paketo-buildpacks/ca-certificates/cacerts" ) -func testLayer(t *testing.T, context spec.G, it spec.S) { +func testTrustedCACerts(t *testing.T, context spec.G, it spec.S) { var ( Expect = NewWithT(t).Expect layerPath string - trustedCAs *cacerts.TrustedCAs + trustedCAs *cacerts.TrustedCACerts certDir string certPaths []string @@ -46,7 +46,7 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { layerPath, err = ioutil.TempDir("", "distribution-layers") Expect(err).NotTo(HaveOccurred()) - trustedCAs = &cacerts.TrustedCAs{ + trustedCAs = &cacerts.TrustedCACerts{ LayerContributor: libpak.NewLayerContributor("CA Certificates", map[string]interface{}{}), GenerateHashLinks: func(dir string, paths []string) error { certDir = dir @@ -76,7 +76,7 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { Expect(layer.BuildEnvironment).To(HaveKey("SSL_CERT_DIR.append")) Expect(layer.BuildEnvironment["SSL_CERT_DIR.append"]). - To(Equal(filepath.Join(layerPath, "certs"))) + To(Equal(filepath.Join(layerPath, "ca-certificates"))) Expect(layer.BuildEnvironment).To(HaveKey("SSL_CERT_DIR.delim")) Expect(layer.BuildEnvironment["SSL_CERT_DIR.delim"]).To(Equal(":")) }) @@ -100,7 +100,7 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { filepath.Join("some-path", "cert1.pem"), filepath.Join("some-path", "cert2.pem"), })) - Expect(certDir).To(Equal(filepath.Join(layerPath, "certs"))) + Expect(certDir).To(Equal(filepath.Join(layerPath, "ca-certificates"))) }) }) } diff --git a/cmd/helper/main.go b/cmd/helper/main.go index e26b439..5211ea8 100644 --- a/cmd/helper/main.go +++ b/cmd/helper/main.go @@ -37,7 +37,7 @@ func main() { cacertHelper := cacerts.NewExecD(bindings) cacertHelper.Logger = bard.NewLogger(os.Stdout) return sherpa.Helpers(map[string]sherpa.ExecD{ - "ca-cert-helper": cacertHelper, + cacerts.ExecutableCACertsHelper: cacertHelper, }) }) }