Add EP-10498: Deflate repository into idiomatic Go layout #10566

timflannagan · 2025-01-31T18:41:11Z

Description

Add an enhancement for deflating the current repository structure (notable the projects/ directory) into a flatter organization. Implementation will follow if accepted.

Related to #10496.
Related to #10498.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

Signed-off-by: timflannagan <[email protected]>

timflannagan · 2025-01-31T19:14:21Z

docs/content/enhancements/10498.md

+- Separate the project's Go applications by extracting existing CLI entry points (currently nested in `projects/*/cmd` subdirectories) into a top-level `cmd/` directory, in order to follow standard Go patterns.
+- Standardize tooling placement by ensuring all project-specific tooling, miscellaneous scripts, and other automation-related files reside in a `hack/` directory.
+
+## Non-Goals


TODO: Add non-goal for separate kgateway/api repository to make it easier for consumers to import that module without the rest of the kgateway module dependencies. This can be done over time if strictly necessary.

yuval-k · 2025-01-31T19:12:10Z

docs/content/enhancements/10498.md

+$ tree -L 1
+├── api/          # Migrated from `projects/gateway2/api/...` initially. Any future API types will be added here.
+├── cmd/          # Top-level directory containing main entry points (e.g., cmd/sds, cmd/controller, cmd/envoy-init, etc.).
+├── internal/     # Private application and library code that is not meant to be imported by external consumers.


yea, i think it makes sense that most code would live here. that way we know we can change it without breaking downstream clients

yuval-k · 2025-01-31T19:14:38Z

docs/content/enhancements/10498.md

+
+```bash
+$ tree -L 1
+├── api/          # Migrated from `projects/gateway2/api/...` initially. Any future API types will be added here.


i've seen projects use the plural form apis because often you have more than one (i.e. v1alpha1, v1beta1, etc..)

Yeah I've seen a combination of api, apis, and pkg/api(s) throughout the ecosystem. I tend to prefer the first two unless the project adopted the pkg/apis organization as k8s tooling matured.

docs/content/enhancements/10498.md

timflannagan · 2025-01-31T20:50:26Z

TODO: what to do about projects/distroless going forward.

npolshakova · 2025-02-05T13:26:22Z

docs/content/enhancements/10498.md

+├── internal/     # Private application and library code that is not meant to be imported by external consumers.
+├── pkg/          # Reusable library code that external consumers could import in the future.
+├── test/         # Test helpers, e2e tests, integration test assets, etc.
+├── hack/         # Houses project-specific tooling, miscellaneous scripts, etc.


We're going to keep hack? :)

other potential names: tools, scripts?

docs/content/enhancements/10498.md

npolshakova · 2025-02-05T13:29:14Z

docs/content/enhancements/10498.md

+├── ci/           # Open question: Should CI-specific scripts remain here or be moved to hack/?
+└── ...
+```
+


Where is https://github.com/kgateway-dev/kgateway/tree/main/install/helm going to live?

That's missing from the current tree layout. I was trying to refactor/rip out/cleanup/etc. the other non-code directories outside of this proposal before enumerating on the final directory structure here. IMO, install/ is fine unless we have better options.

(Added the install directory to the tree output in the latest round of commits.)

This commit temporarily removes the CODEOWNERS file now that the docs/ directory has largely been migrated to the kateway.dev repository. Previously, this file contained minimal code owner paths and tagged the docs team whenever any non-osa_*.md file(s) were changed in the docs/ directory. Related to kgateway-dev#10566 which overhauls the repository structure. We can re-evaluate the right CODEOWNERS approach once that EP has been approved and implemented. Related to kgateway-dev#10431 which tracks better CODEOWNER adoption throughout the repository. Signed-off-by: timflannagan <[email protected]>

…n-proposal

Signed-off-by: timflannagan <[email protected]>

timflannagan · 2025-02-05T17:35:06Z

design/10498.md

+├── api/          # Migrated from `projects/gateway2/api/...` initially. Any future API types will be added here.
+├── cmd/          # Top-level directory containing main entry points (e.g., cmd/sds, cmd/controller, cmd/envoy-init, etc.).
+├── design/       # Enhancements, templates, etc.
+├── examples/     # Example usage, etc. Open question on whether we want to keep this. The README.md would reference this directory.


Leaning towards removing this directory. See #10579 (review) for more context.

From ^ PR ref:

does it make sense to just remove the examples directory and rely on the unit / e2e tests as examples? in theory the tests should cover (most of) the use cases we support

Consider using examples for quickstart guide assets (xref).

timflannagan · 2025-02-05T17:37:07Z

design/10498.md

+### Proposed Directory Layout
+
+```bash
+$ tree -L 1


The docs/ directory is missing from this proposed directory layout. See #10576 which overhauled the docs/ directory and replaced the contents with a docs/README.md file that references the dedicated kgateway.dev documentation repository. I think we're leaning towards removing that directory as a follow-up after this EP is implemented.

design/10498.md

jenshu · 2025-02-06T15:09:21Z

design/10498.md

+
+- `internal/` usage: Which packages are truly internal-only vs. possibly importable by external consumers?
+- Versioning: Should there be a dedicated `internal/version` package, or is placing version info in `cmd/` acceptable?
+- CI directory: Should CI-specific scripts remain in `ci/` or be moved into `hack/`?


it seems like most scripts under ci can also be used for local debugging so i wouldn't be opposed to moving it into hack. what is the convention for this in other projects? we can also leave them separate for now and iterate on it later

I think hack is pretty widely adopted throughout the k8s ecosystem, so I'm leaning towards consolidating the ci/* contents into the hack directory.

+1 You're spot on @timflannagan with using hack and we can create add'l sub dirs from there as needed.

jenshu · 2025-02-06T15:11:15Z

design/10498.md

+- Versioning: Should there be a dedicated `internal/version` package, or is placing version info in `cmd/` acceptable?
+- CI directory: Should CI-specific scripts remain in `ci/` or be moved into `hack/`?
+- projects/distroless: Should we keep this directory? If so, where should it live?
+- projects/gateway2: Do we need to keep the non-code files, e.g. istio.sh, Makefile, etc.?


afaik istio.sh is for local testing, can probably be moved into the hack directory

we should probably combine this Makefile into the top-level Makefile

Add EP-10498: Deflate repository into idiomatic Go layout

2a8c1ba

Signed-off-by: timflannagan <[email protected]>

timflannagan commented Jan 31, 2025

View reviewed changes

yuval-k reviewed Jan 31, 2025

View reviewed changes

timflannagan commented Jan 31, 2025

View reviewed changes

docs/content/enhancements/10498.md Outdated Show resolved Hide resolved

timflannagan mentioned this pull request Feb 3, 2025

docs: Migrate documentation to the kgateway.dev repository #10576

Merged

4 tasks

npolshakova reviewed Feb 5, 2025

View reviewed changes

docs/content/enhancements/10498.md Outdated Show resolved Hide resolved

npolshakova reviewed Feb 5, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main' into design/add-deflatio…

8f2f541

…n-proposal

timflannagan mentioned this pull request Feb 5, 2025

projects/gw2: Move examples to root directory #10579

Merged

4 tasks

timflannagan added 2 commits February 5, 2025 16:32

design: Update 10498.md proposal

303e124

Signed-off-by: timflannagan <[email protected]>

design: Flesh out the rest of the root directories

e63274f

Signed-off-by: timflannagan <[email protected]>

timflannagan commented Feb 5, 2025

View reviewed changes

design/10498.md Show resolved Hide resolved

timflannagan mentioned this pull request Feb 5, 2025

*: Flatten repository structure #10583

Merged

4 tasks

jenshu reviewed Feb 6, 2025

View reviewed changes

jenshu approved these changes Feb 6, 2025

View reviewed changes

danehans approved these changes Feb 6, 2025

View reviewed changes

danehans added this pull request to the merge queue Feb 6, 2025

Merged via the queue into kgateway-dev:main with commit d3308f7 Feb 6, 2025
8 checks passed

timflannagan deleted the design/add-deflation-proposal branch February 6, 2025 21:05

timflannagan mentioned this pull request Feb 7, 2025

Consolidate the ci/ and hack/ directories #10589

Merged

4 tasks

stevenctl pushed a commit to stevenctl/gloo that referenced this pull request Feb 17, 2025

[1.18] 6748-spawn-upstream-span-backport (kgateway-dev#10566)

5d4f634

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EP-10498: Deflate repository into idiomatic Go layout #10566

Add EP-10498: Deflate repository into idiomatic Go layout #10566

timflannagan commented Jan 31, 2025

timflannagan Jan 31, 2025

yuval-k Jan 31, 2025

yuval-k Jan 31, 2025 •

edited

Loading

timflannagan Jan 31, 2025

timflannagan commented Jan 31, 2025

npolshakova Feb 5, 2025

jenshu Feb 6, 2025

npolshakova Feb 5, 2025

timflannagan Feb 5, 2025

timflannagan Feb 5, 2025

timflannagan Feb 5, 2025

danehans Feb 6, 2025

timflannagan Feb 5, 2025

jenshu Feb 6, 2025

timflannagan Feb 6, 2025

danehans Feb 6, 2025

jenshu Feb 6, 2025

Add EP-10498: Deflate repository into idiomatic Go layout #10566

Add EP-10498: Deflate repository into idiomatic Go layout #10566

Conversation

timflannagan commented Jan 31, 2025

Description

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuval-k Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Jan 31, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuval-k Jan 31, 2025 •

edited

Loading