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.

Sign up for GitHub

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

decorrelate source and target directories #282

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+118 −0
Open

decorrelate source and target directories #282

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions text/0000-decorrelate-source-targer-dirs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# Meta
[meta]: #meta
- Name: Decorrelate source dans target directories
- Start Date: 2023-04-03
- Author(s): @xfrancois
- Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded -->
- RFC Pull Request: (leave blank)
- CNB Pull Request: (leave blank)
- CNB Issue: [Can't make final image directory different from the CNB_APP_DIR](https://github.com/buildpacks/lifecycle/issues/1034)
- Supersedes: N/A

# Summary
[summary]: #summary

Allow CNB users to build image from one directory and have the final image directory structure to be something else. Currently it's possible to change the default target directory (/workspace) by setting the `CNB_APP_DIR` environement variable, but the source directory must also be the one specified in `CNB_APP_DIR`. This behavior is not well suited for constrained environements, like CI.
Copy link
Contributor

Choose a reason for hiding this comment

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

this is almost a side-note, but the lifecycle exporter also takes a cmd line arg -app that can override that same CNB_APP_DIR . There's also a corresponding flag in pack, --path or -p for overriding the same. This might not be important from the spec perspective, but from an implementation perspective there's actually several things, that already have inconsistent naming, to separate and find new names for.


# Definitions
[definitions]: #definitions

N/A
Copy link
Contributor

Choose a reason for hiding this comment

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

imo it would be helpful here to define

"the place where the application source lives"

and

"the place where we will put the built artifacts after building the application"

Right now, both are referred to by the same name, but also both have 3 names (per the above list of flags and env vars). so i think it will be helpful for this document to find names that work, as those could help lead to unified flag and env var naming as well :)


# Motivation
[motivation]: #motivation

- Why should we do this?

We should do this to provide more flexibility for end-users.

- What use cases does it support?

Building from a CI tool, for example Gitlab-CI. We can't chose the source directory from which we are building. On Gitlab-CI the Git folder is checkout on `/builds/<group>>/<project>`. By having a way to decorrelate source and target directories, we could chose the finaly directory structure we want.
Copy link
Contributor

Choose a reason for hiding this comment

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

first a nit: there's a couple typos here:

we could chose the finaly directory structure
should probably be:
"we could choose the final directory structure"

but more importantly, I still don't fully understand the why of this proposal. What do you gain by being able to choose your directory structure? What gets easier for you? What is blocked by a lack of this ability currently?


- What is the expected outcome?

Support of a new environment variable / parameter that allows to set the source directory. We probably should document that `CNB_APP_DIR` only handles the target directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good, just see above as there seems to be 3 touch points not just the env var.


| Variable | Default value | Description |
|--------------------|---------------|----------------------------|
| `CNB_APP_DIR` | `/workspace` | Target directory structure |
| `CNB_SOURCE_APP_DIR` | `$CNB_APP_DIR` | Source directory structure |

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

This provides a high level overview of the feature.

- Define any new terminology.
- Define the target persona: buildpack author, buildpack user, platform operator, platform implementor, and/or project contributor.
- Explaining the feature largely in terms of examples.
- If applicable, provide sample error messages, deprecation warnings, or migration guidance.
- If applicable, describe the differences between teaching this to existing users and new users.

Comment on lines +45 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this part is just the boilerplate? should probably be deleted or filled in with your specific case :)

# How it Works
[how-it-works]: #how-it-works

In the Gitlab-CI example, I can't chose the source directory but I want to be able to chose the target directory. I will set the variables like this :

`CNB_SOURCE_APP_DIR`: /builds/group/project
Copy link
Contributor

Choose a reason for hiding this comment

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

given the way the appPath is currently used in pack it may be more backwards compatible to keep the "APP_DIR" as the variable which means "both source and destination"
OR as the variable which means "source only" rather than as the variable that means "destination only".

Looking in lifecycle it also looks like it's pretty thoroughly "the source and destination" directory, so i wonder if it would be better to add 2 entirely new env vars and then do some validation that they're used exclusively with the existing env vars and flags?


`CNB_APP_DIR`: /workspace

The final image will be available

# Migration
[migration]: #migration

N/A

# Drawbacks
[drawbacks]: #drawbacks

Copy link
Contributor

Choose a reason for hiding this comment

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

it's a little disingenuous to claim there's no drawbacks -- at the very least, any new variable is something that comes with a small maintenance and complexity cost.

N/A

# Alternatives
[alternatives]: #alternatives

- What is the impact of not doing this?

The impact is that people wanting to use CNB on CI environment will have to make some workaround (like copying source folders in another location) in order to have a proper directory structure in the final image.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the impact of having an "improper" directory structure? What gets harder or worse other than an abstract sense of "proper-ness" ?

Copy link
Author

@xfrancois xfrancois Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's a bit complicated to explain but let me try :
On Gitlab-CI I'm building an application A that uses an app B as a "service" (equivalent to docker-compose in gitlab-ci).
This app B in my context has also been built with buildpack on Gitlab-CI. As it's been build on Gitlab-CI, by default the working directory is /workspaces, so we need to set up the CNB_APP_DIR to /worspaces. Then the final image of the application B would have a directory of /workspaces.

Back to the application A build : when launching the app B as a service, the directory structure of the B app will clash with Gitlab-CI default, and then the app B won't be able to run.

To sum up, the inability to chose the "proper" directory structure prevents us from running my app as a service dependency in Gitlab-CI context.

The only workaround that works is when building the "B" app, copy the content from /worskpaces (default Gitlab CI folder) to another location, set the CNB_APP_DIR to this other location. That way the final directory structure will not be "/workspaces" and it won't clash when loading this app as a service when building app A.

I have also documented this issue here : https://stackoverflow.com/a/75875777/3767820
Hope the need is clearer

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @xfrancois - i think it's coming into focus but it's worth really understanding.

This situation arises when:

  • you are invoking the lifecycle directly, so you're managing your own "unit of hermetic building" via CI. (If you were using pack, the 'hermetic unit' would be the container created by pack, which is fresh for each invocation)

  • you want to build 2 (coupled) applications in the same "hermetic unit"; i.e. the build and run for apps A and B share the same filesystem.

  • you want two different images / or "runnable units"

  • you are constrained by your "provider of hermetic units" (your CI) to have a hardcoded directory name

  • and the problem is then that you wind up with two applications in a single directory?

  • and the current workaround is to manually copy a directory?

To be honest i'm still a little confused. Maybe you can draw a more detailed sketch. I'm not finding the word "proper" helpful as I think you're referring to a convention that I don't know, so it might help to spell out that convention?

Are you trying to wind up with one image that can run either app A or B?

Why wouldn't it work to do something like

cp -r path/to/appA/src /workspaceA
lifecycle ... -app /workspaceA
cp -r path/to/appB/src /workspaceB
lifecycle ... -app /workspaceB

?

Copy link
Member

Choose a reason for hiding this comment

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

@xfrancois any further thoughts here?


# Prior Art
[prior-art]: #prior-art

Discuss prior art, both the good and bad.

# Unresolved Questions
[unresolved-questions]: #unresolved-questions

- What parts of the design do you expect to be resolved before this gets merged?
- What parts of the design do you expect to be resolved through implementation of the feature?
- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?

# Spec. Changes (OPTIONAL)
[spec-changes]: #spec-changes
Does this RFC entail any proposed changes to the core specifications or extensions? If so, please document changes here.
Examples of a spec. change might be new lifecycle flags, new `buildpack.toml` fields, new fields in the buildpackage label, etc.
This section is not intended to be binding, but as discussion of an RFC unfolds, if spec changes are necessary, they should be documented here.

# History
[history]: #history

<!--
## Amended
### Meta
[meta-1]: #meta-1
- Name: (fill in the amendment name: Variable Rename)
- Start Date: (fill in today's date: YYYY-MM-DD)
- Author(s): (Github usernames)
- Amendment Pull Request: (leave blank)

### Summary

A brief description of the changes.

### Motivation

Why was this amendment necessary?
--->