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

Project Descriptor Domains RFC #140

Merged
merged 8 commits into from
Apr 27, 2021

Conversation

hone
Copy link
Member

@hone hone commented Mar 2, 2021

No description provided.

@hone hone requested a review from a team as a code owner March 2, 2021 18:13
@hone hone marked this pull request as draft March 2, 2021 18:13
Signed-off-by: Terence Lee <hone02@gmail.com>
@hone hone force-pushed the project-descriptor-domains branch from 84d8e04 to 5f11bab Compare March 2, 2021 20:47
hone added 2 commits March 2, 2021 19:10
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
@hone hone force-pushed the project-descriptor-domains branch from 48a54ff to b4ac250 Compare March 3, 2021 01:27
@hone hone marked this pull request as ready for review March 3, 2021 16:39
@hone
Copy link
Member Author

hone commented Mar 3, 2021

Feedback from @jromero during the platform subteam sync:

  • add api field under [project] or the top level
  • clarify that api field is optional for reverse domain namespaces

@hone
Copy link
Member Author

hone commented Mar 3, 2021

From WG today: need to solve the top level domain issue. By using [project], it squats on the ".project" TLD. _ can be used in bare TOML keys that aren't allowed in domain names.

@jkutner
Copy link
Member

jkutner commented Mar 10, 2021

I propose special casing the project TLD to resolve the issue.

  1. We forbid the project table from being customized (thus, you cannot add keys to [project] or [project.mydomain]
  2. As an escape hatch, we allow ["project.mydomain"] and interpret it as project.mydomain table.

Optionally, we forbid all TLD tables. This means that an org owning .google cannot create [google] key = "value", but it prevents confusion when [google] is alongside [project]

@hone
Copy link
Member Author

hone commented Mar 11, 2021

@jkutner as discussed in the WG, forbidding just TLDs doesn't really solve the problem b/c what if I have [project.foo]. We're just squatting on the entire TLD.


This suprsedes the [Project Descriptor Flexibility RFC](https://github.com/buildpacks/rfcs/pull/95).

## Special Case Project Namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, we could define that the root level of this TOML does not use reverse domain keys and move them under a generic config (or similar) key. This would allow us to use descriptive keys for things that are generic (such as [project]) and still allow folks using the .project TLD to use this feature without workarounds.

This would also allow us to keep the build key if we'd like to keep it.

Example:

[project]
id = ""
name = ""
# ...

[config.com.example]
awesome = "sauce"

I'm not sure how much I personally like this, but it seems to be a viable alternative.

Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
@hone hone force-pushed the project-descriptor-domains branch from e5cd6fb to 952bee5 Compare March 17, 2021 18:52
Signed-off-by: Terence Lee <hone02@gmail.com>
@nebhale nebhale requested a review from a team March 24, 2021 16:26
@jromero
Copy link
Member

jromero commented Mar 31, 2021

@buildpacks/platform-maintainers - please create issues for this. (update parsing of project.toml)

@sclevine
Copy link
Member

sclevine commented Apr 7, 2021

@buildpacks/implementation-maintainers @buildpacks/distribution-maintainers needs issues

@jromero
Copy link
Member

jromero commented Apr 8, 2021

@nebhale I see you added the issues-created/core but not finding a spec issue. If I'm not mistaken this should have one since it's updating the project descriptor spec, right?

@sclevine
Copy link
Member

@jromero to create issue and merge

jkutner added a commit that referenced this pull request Apr 27, 2021
[#140]

Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@jkutner jkutner merged commit 78470e9 into buildpacks:main Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants