Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Override Env Vars By Default #98

Merged
merged 3 commits into from
Aug 11, 2020
Merged

RFC: Override Env Vars By Default #98

merged 3 commits into from
Aug 11, 2020

Conversation

sclevine
Copy link
Member

@sclevine sclevine commented Jul 27, 2020

Signed-off-by: Stephen Levine <stephen.levine@gmail.com>
@sclevine sclevine requested a review from a team as a code owner July 27, 2020 04:11
text/0000-override-env-by-default.md Outdated Show resolved Hide resolved
text/0000-override-env-by-default.md Outdated Show resolved Hide resolved
Signed-off-by: Stephen Levine <stephen.levine@gmail.com>

Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>
@nebhale nebhale requested a review from a team July 27, 2020 16:58
@nebhale nebhale requested a review from a team July 27, 2020 16:59
Signed-off-by: Stephen Levine <stephen.levine@gmail.com>

Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>
@nebhale nebhale requested a review from a team August 3, 2020 14:38
@nebhale
Copy link
Contributor

nebhale commented Aug 3, 2020

Final Comment Period with merge disposition, closing on 10 August, 2020.

# What it is
[what-it-is]: #what-it-is

Buildpack authors can create environment variable files as `<layer>/env(.launch|.build|)/NAME` such the an environment variable with name `NAME` is exported into the runtime, build-time (for subsequent buildpacks), or both environments. The contents of the file are set as the value.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Buildpack authors can create environment variable files as `<layer>/env(.launch|.build|)/NAME` such the an environment variable with name `NAME` is exported into the runtime, build-time (for subsequent buildpacks), or both environments. The contents of the file are set as the value.
Buildpack authors can create environment variable files as `<layer>/env(.launch|.build|)/NAME` such that an environment variable with name `NAME` is exported into the runtime, build-time (for subsequent buildpacks), or both environments. The contents of the file are set as the value.


Buildpack authors can create environment variable files as `<layer>/env(.launch|.build|)/NAME` such the an environment variable with name `NAME` is exported into the runtime, build-time (for subsequent buildpacks), or both environments. The contents of the file are set as the value.

These environment variable files are applied dynamically, either before subsequent buildpacks or when the app container starts. Their application may be modulated by file extensions such as `.append`, `.prepend`, `.default`, and `.override`. For `.append` and `.prepend`, a special `NAME.delim` file may be used to specify the delimiter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify – if you wanted to append to PATH, you would create a file <layer>/env(.launch|.build|)/NAME.append?

@hone
Copy link
Member

hone commented Aug 5, 2020

Overall, we should make this change but the devil is in handling the backwards breaking changes. I assume this is slated for 1.0. Do we plan to handle the current behavior and this behavior in the same group?

@nebhale
Copy link
Contributor

nebhale commented Aug 5, 2020

@hone If we can find a comfortable way to do it, I would say yes. Some options might be:

  1. Include Buildpack API Version information in the built image metadata such that the lifecycle can behave differently as the environment is bootstrapped.
  2. Rewrite the filenames during export to match the "canonical" names for the behavior based on the inputs at build time.

I think I like 2 best, but it feels like we've got some options to play around with that should give us the compatibility we desire.

@sclevine
Copy link
Member Author

sclevine commented Aug 5, 2020

Include Buildpack API Version information in the built image metadata such that the lifecycle can behave differently as the environment is bootstrapped.

I think @ekcasey had plans to do this already.

nebhale added a commit that referenced this pull request Aug 11, 2020
[#98]

Signed-off-by: Ben Hale <bhale@vmware.com>
@nebhale nebhale merged commit c1a6bda into main Aug 11, 2020
@nebhale nebhale deleted the override-env-default branch August 11, 2020 22:52
nebhale added a commit to buildpacks/libcnb that referenced this pull request Sep 10, 2020
Given an upcoming spec change around how environment variables are written to
the filesystem, this change updates the API to be compatible with both old and
new versions.  It does this by eliminating the "unqualified" API and requiring
explicit declarations for all API usage.  It also eliminates the "delimiter"
APIs in favor of requiring a delimiter for all "append" and "prepend" APIs.

[buildpacks/spec#131][buildpacks/rfcs#98]

Signed-off-by: Ben Hale <bhale@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants