Buildpack is converted to a supply buildpack #19

izgeri · 2019-02-06T19:44:39Z

Connected to #17

In order to convert our buildpacker to a supply buildpack, this PR moves the buildpack installation logic into a single supply script and changes the directories in which dependencies are installed, along with a few other changes to support this shift. It also cleans up tests a bit and adds comments where useful, and updates the conjur-env binary to use updated dependencies and Go modules.

This commit is the first step toward converting toward a supply buildpack. Known issues: - Running cf create-buildpack fails because the bin/compile script is missing, but it's not clear what this (deprecated) script is supposed to do - The bin/supply script is actually only allowed to modify the deps and index directories, and it's now set to modify the build directory. That will have to be updated, and we will need to determine how the retrieve secrets script will get run by the app. We may also need to update how the bp-supplied scripts / conjur-env look for secrets.yml, if they are no longer working from the same directory as the app dir.

izgeri · 2019-02-07T02:24:17Z

Notes on the work remaining:

The buildpack will need a bin/compile script even though it's deprecated
- From Pivotal, there is currently a bug (I suspect it's from here) that expects any buildpack to have a bin/compile script. We will need to add one - it's fine if it just exits like this one
The bin/detect script should be deleted, and its check for the secrets.yml file should be moved to bin/supply
- The detect script is likely not required, and if we supply it it's possible the language buildpack won't get used (eg if the user doesn't specify a buildpack, and bin/detect returns true for the app). Its functionality (checking for secrets.yml) should likely be moved to the bin/supply script. See the CF buildpack detection docs for more info.
The bin/supply script must be updated to copy files to DEPS_DIR - writing to other directories will fail
- Currently it works just like bin/compile used to and puts files in BUILD_DIR, but that will probably fail in practice
- It is unknown how to force 0001_retrieve-secrets.sh to run on app start if we can't put it in BUILD_DIR/.profile_d/
- It is unknown where the script needs to look for secrets.yml if it is not itself running from the BUILD_DIR
- It looks like files copied to DEPS_DIR will end up in dep/ - if you cf ssh to the app, you should see app/ (with all the app files) and dep/ (with all the supply buildpack dependencies). From the docs:
  
  At the end of this process, the deps directory is included at the root of the droplet, adjacent to the app directory.
- This documentation on the custom buildpack contract may help to clarify some of these details - it looks like items copied into DEPS_DIR/.profile_d by the supply buildpack will be copied into the app's .profile_d and invoked at runtime like before, but it's not clear where to put the conjur-env binary (we were putting it in vendor before)

micahlee

@jtuttle, this looks good from reading the changes. I just had a couple of small comments, and it looks like this needs README and CHANGELOG updates still.

This is probably an issue for the future and not this PR, but it would probably be worth eventually adding a live integration test to this repo as well to verify the behavior in PCF as part of the CI.

micahlee · 2019-02-14T15:29:03Z

bin/compile

-  cp ${BUILDPACK_DIR}/vendor/conjur-env ./vendor/conjur-env
-  cp ${BUILDPACK_DIR}/lib/0001_retrieve-secrets.sh ./.profile.d/0001_retrieve-secrets.sh
-popd
+exit


Would you mind adding the newline here at the end?

micahlee · 2019-02-14T15:30:37Z

bin/supply

+#!/bin/bash -e
+# bin/supply <build-dir> <cache-dir> <deps-dir> <index>
+
+BUILD_DIR=$1


It would be helpful to add a quick comment for each of the directories and their purpose/role, either here or in the README.

Added comments.

izgeri · 2019-02-14T16:22:02Z

Added #20 for adding automated tests to run against the live PCF instance

izgeri · 2019-02-14T16:24:04Z

ci/features/buildpackless.feature

-    Given the compile script is run against the app's root folder
+    Given the build directory has a secrets.yml file
+    And VCAP_SERVICES contains cyberark-conjur credentials
+    And the supply script is run against the app's root folder


Should we also have two tests that validate what happens when

the build dir does not have secrets.yml, and/or

VCAP_SERVICES does not contain cyberark-conjur creds?

These conditions are already tested in the supply.feature so I think we're okay.

izgeri · 2019-02-14T16:24:38Z

ci/features/compile.feature

@@ -1,9 +0,0 @@
-Feature: Compile script


Should we leave a test here that validates that this script exits for now? I could go either way.

I'd say let's just leave it out. It's such a simple script that if anything goes wrong it'd probably be something not covered by the test we would right.

izgeri · 2019-02-14T16:25:34Z

ci/features/profile_d.feature

-    Given the 'compile' script is run
+    Given the build directory has a secrets.yml file
+    And VCAP_SERVICES contains cyberark-conjur credentials
+    And the supply script is run against the app's root folder


Same comment here, should we test when the conditions are not right?

Hm this feature appears to be identical to the buildpackless one. They use different step definitions on master, but they essentially boil down to the same thing. I think we should probably just remove the buildpackless one.

izgeri · 2019-02-14T16:27:08Z

ci/features/step_definitions/profile_d_steps.rb

@@ -1,17 +1,10 @@
 When(/^the retrieve secrets \.profile\.d script is sourced$/) do
  @commands ||= []
  @commands << <<EOL
-. #{@BUILD_DIR}/.profile.d/0001_retrieve-secrets.sh #{@BUILD_DIR}
+HOME=#{@BUILD_DIR} DEPS_DIR=#{@DEPS_DIR} . #{@DEPS_DIR}/#{@INDEX_DIR}/.profile.d/0001_retrieve-secrets.sh


Can we add a comment here explaining the reason for passing in HOME / DEPS_DIR and the syntax being used?

Yep, added a comment.

izgeri · 2019-02-14T16:30:02Z

lib/0001_retrieve-secrets.sh

-# inject secrets into environment
-pushd $1
-  eval "$(./vendor/conjur-env)"
+# $HOME points to the app directory, which should contains a secrets.yml file.


Is there documentation we can link to that will share how we know HOME and DEPS_DIR will be set when this script is run?

Unfortunately no. This is knowledge I gained from the examples linked to in Slack conversations (and the conversations themselves). This seems to be quite poorly documented.

izgeri · 2019-02-14T17:03:33Z

@jtuttle can you revise the PR description so that it has more info about what the changes contain? might be helpful if looking back at this PR in the future

jtuttle · 2019-02-14T18:40:54Z

@izgeri PR description updated.

izgeri

LGTM. @jtuttle do you want to organize the commits before merge?

izgeri · 2019-02-14T22:03:26Z

README.md

@@ -1,4 +1,4 @@
-The CyberArk Conjur Buildpack is a [decorator buildpack](https://github.com/cf-platform-eng/meta-buildpack#what-is-a-decorator) that provides convenient and secure access to secrets stored in Conjur.
+The CyberArk Conjur Buildpack is a [supply buildpack](https://docs.cloudfoundry.org/buildpacks/custom.html#contract) that provides convenient and secure access to secrets stored in Conjur.

 The buildpack carries out the following:


I think more accurately the buildpack installs some scripts that do the following

izgeri · 2019-02-14T22:05:47Z

README.md

-### Lifecycle scripts
-
-The buildpack is comprised of the [3 lifecycle scripts](https://github.com/cf-platform-eng/meta-buildpack#how-to-write-a-decorator) that are required for decorator buildpacks.
+The buildpack uses a [supply script](https://docs.cloudfoundry.org/buildpacks/custom.html#contract) to copy files into the application's dependency directory under a subdirectory corresponding to the buildpack's index. The `lib/0001_retrieve-secrets.sh` script is copied into a `.profile.d` subdirectory so that it will run automatically when the app starts and the `conjur-env` binary is copied to a `vendor` subdirectory. In other words, your application will end up with the following two files:


https://docs.cloudfoundry.org/buildpacks/understand-buildpacks.html#supply-script is a better link here

izgeri · 2019-02-14T22:07:05Z

README.md


-The `.profile.d` script is responsible for retrieving secrets and injecting them into the session environment variables at the start of the app.
+The `.profile.d` script is run automatically when the application starts and is responsible for retrieving secrets and injecting them into the app's session environment variables.



Maybe worth adding something here about how the conjur-env binary leverages Conjur's Go API to open a connection to Conjur and retrieve secrets

izgeri · 2019-02-14T22:09:15Z

README.md

-
-+ copy the contents of the `./lib` directory in a release to the `./.profile.d` directory relative to your app's root directory.
-+ copy the contents of the `./vendor` directory in a release to the `./vendor` directory relative to your app's root directory.
-
 When your application starts the secrets specified in the `secrets.yml` file will now be available in the session environment variables at the start of the app.



We may still have users who need to use the decorator buildpack functionality, and they can do so using the 1.0.0 release of the buildpack. I feel pretty strongly that we should continue to include those instructions somewhere - can we either do that here, or log an issue in the conjur-docs to add documentation for using the original meta-buildpack functionality to the (non-existent yet) CF buildpack documentation?

Issue #21 will address this

- Describe new supply buildpack functionality - Organize / improve usage information - Organize / improve development information

micahlee · 2019-02-15T19:07:41Z

CONTRIBUTING.md

+
+## Prerequisites
+
+Before getting started, you should install some developer tools. These are not required to deploy the Conjur Service Broker but they will let you develop using a standardized, expertly configured environment.


micahlee · 2019-02-15T19:10:43Z

CONTRIBUTING.md

+
+### Releasing
+
+1. Based on the unreleased content, determine the new version number and update the [VERSION](VERSION) file.


Should there be a mention here about semver, or what it means to determine the new version number?

- Update the conjur-api-go and summon versions - Convert to go modules

micahlee

LGTM!

ghost assigned izgeri Feb 6, 2019

ghost added review implementing labels Feb 6, 2019

add empty compile script

c111b2b

ghost assigned jtuttle Feb 8, 2019

John Tuttle added 3 commits February 8, 2019 11:04

remove bin/detect and move secrets.yml check to bin/supply

c173b80

remove detect cuke and move test to supply cuke

c087ae0

remove duplicate and unused step definitions

be53ec3

jtuttle force-pushed the supply-buildpack branch from e0b9cd6 to be53ec3 Compare February 13, 2019 22:21

jtuttle changed the title ~~WIP - Initial conversion to supply buildpack~~ Initial conversion to supply buildpack Feb 13, 2019

micahlee self-requested a review February 14, 2019 14:31

micahlee reviewed Feb 14, 2019

View reviewed changes

izgeri commented Feb 14, 2019

View reviewed changes

remove buildpackless test as duplicate, add comments

8df828a

izgeri commented Feb 14, 2019

View reviewed changes

cyberark deleted a comment from jtuttle Feb 14, 2019

jtuttle mentioned this pull request Feb 14, 2019

Docs includes instructions for decorator buildpack #21

Open

Update README to replace meta-buildpack info

615bdbc

- Describe new supply buildpack functionality - Organize / improve usage information - Organize / improve development information

izgeri force-pushed the supply-buildpack branch 2 times, most recently from 6be89d7 to 23f5f99 Compare February 15, 2019 17:57

izgeri changed the title ~~Initial conversion to supply buildpack~~ Buildpack is converted to a supply buildpack Feb 15, 2019

micahlee reviewed Feb 15, 2019

View reviewed changes

izgeri force-pushed the supply-buildpack branch 2 times, most recently from 3b0c2f8 to e1b3036 Compare February 15, 2019 19:51

Geri Jennings added 3 commits February 15, 2019 14:53

Move developer content to CONTRIBUTING.md

7fb322e

Update conjur-env dependencies

9a2076a

- Update the conjur-api-go and summon versions - Convert to go modules

Bump version to 2.0.0

120b145

izgeri force-pushed the supply-buildpack branch from e1b3036 to 120b145 Compare February 15, 2019 19:53

micahlee approved these changes Feb 15, 2019

View reviewed changes

micahlee merged commit 7bccc30 into master Feb 15, 2019

ghost removed the implementing label Feb 15, 2019

micahlee deleted the supply-buildpack branch February 15, 2019 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Buildpack is converted to a supply buildpack #19

Buildpack is converted to a supply buildpack #19

izgeri commented Feb 6, 2019 •

edited

Loading

izgeri commented Feb 7, 2019 •

edited

Loading

micahlee left a comment

micahlee Feb 14, 2019

micahlee Feb 14, 2019

jtuttle Feb 14, 2019 •

edited

Loading

izgeri commented Feb 14, 2019

izgeri Feb 14, 2019

jtuttle Feb 14, 2019

izgeri Feb 14, 2019

jtuttle Feb 14, 2019

izgeri Feb 14, 2019

jtuttle Feb 14, 2019

izgeri Feb 14, 2019

jtuttle Feb 14, 2019

izgeri Feb 14, 2019

jtuttle Feb 14, 2019 •

edited

Loading

izgeri commented Feb 14, 2019

jtuttle commented Feb 14, 2019

izgeri left a comment

izgeri Feb 14, 2019

izgeri Feb 14, 2019 •

edited

Loading

izgeri Feb 14, 2019

izgeri Feb 14, 2019

izgeri Feb 15, 2019

micahlee Feb 15, 2019

micahlee Feb 15, 2019

micahlee left a comment


		The `.profile.d` script is responsible for retrieving secrets and injecting them into the session environment variables at the start of the app.
		The `.profile.d` script is run automatically when the application starts and is responsible for retrieving secrets and injecting them into the app's session environment variables.


		## Prerequisites

		Before getting started, you should install some developer tools. These are not required to deploy the Conjur Service Broker but they will let you develop using a standardized, expertly configured environment.


		### Releasing

		1. Based on the unreleased content, determine the new version number and update the [VERSION](VERSION) file.

Buildpack is converted to a supply buildpack #19

Buildpack is converted to a supply buildpack #19

Conversation

izgeri commented Feb 6, 2019 • edited Loading

izgeri commented Feb 7, 2019 • edited Loading

micahlee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtuttle Feb 14, 2019 • edited Loading

Choose a reason for hiding this comment

izgeri commented Feb 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtuttle Feb 14, 2019 • edited Loading

Choose a reason for hiding this comment

izgeri commented Feb 14, 2019

jtuttle commented Feb 14, 2019

izgeri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izgeri Feb 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

micahlee left a comment

Choose a reason for hiding this comment

izgeri commented Feb 6, 2019 •

edited

Loading

izgeri commented Feb 7, 2019 •

edited

Loading

jtuttle Feb 14, 2019 •

edited

Loading

jtuttle Feb 14, 2019 •

edited

Loading

izgeri Feb 14, 2019 •

edited

Loading