build: setup contrib

[mattklein123]

Also move Squash extension to contrib

As per: https://docs.google.com/document/d/1yl7GOZK1TDm_7vxQvt8UQEAu07UQFru1uEKXM6ZZg_g/edit#

Part of #14078

Risk Level: Low
Testing: Existing tests. Also a bunch of manual testing.
Docs Changes: Added
Release Notes: Added
Platform Specific Features: N/A

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #17595 was opened by mattklein123.

see: more, trace.

[mattklein123]

@phlax @lizan I have more work to do on documentation but I think this is pretty close to ready on the code side. Can you give me a first pass to see if you have any high level comments while I work on more docs?

[mattklein123]

@phlax @htuch @adisuissa @lizan this is ready for review. PTAL. The only thing not done is the creation of the docker hub repos which I will do once we agree on the general shape of this.

[lizan]

I thought this will be contrib/squash/source/filters/http/config.h? Per https://docs.google.com/document/d/1yl7GOZK1TDm_7vxQvt8UQEAu07UQFru1uEKXM6ZZg_g/edit#heading=h.zgixgx1c4t8u

[mattklein123]

IMO it makes more sense to use the full extension path that matches the normal extensions tree. I can make the doc more clear on this.

[lizan]

I meant we want contrib/<extension_name>/ as a unit, have source and test under it. In that way we can group extensions that have to be used together (for example, something like dynamic_forward_proxy should have both filter and cluster extension under contrib/dynamic_forward_proxy.) It also makes CODEOWNER clearer instead of mixing multiple extensions in same tree.

[mattklein123]

I did end up grouping source/test under the same name, but I still think if we just do things by name it will get pretty chaotic. I have a strong preference for matching the same directory structure as source/extensions. You don't think it will get extremely messy if we don't have this structure?

[lizan]

I did end up grouping source/test under the same name, but I still think if we just do things by name it will get pretty chaotic. I have a strong preference for matching the same directory structure as source/extensions. You don't think it will get extremely messy if we don't have this structure?

No, that was the whole point I proposed contrib/<extension_name> and I feel strongly that under every contrib/extension_name it should look like a standalone repository. Then we can give the CODEOWNERS responsibility of managing everything under it, if no one is maintaining we can just delete the whole directory. It also has the flexibility of having multiple extension under same name like I stated above. We can have a guidance that under contrib/<extension_name> should still follow same structure of the main tree.

[alyssawilk]

I mildly prefer keeping code for a given extension in a given directory, though deleting the contrib/source/extensions/foo contrib/include/extensions/foo contrib/test/extensions/foo isn't a deal breaker (I'm fine either way)

I'm strongly against copy-pasted common code. I'd lean towards a common directory with default visibility private, and explicit opt-in for each library so we can easily tell that common/foo_lib is used by extension A and B, and clean up if we delete extension A and B.

[mattklein123]

Sorry I want to be completely clear about what is being proposed here since I'm not sure everyone is talking about the same thing.

The current code:

/contrib/<... full hierarchical path based on extension type ...>/<extension name>/<source|test>/

Lizan's proposal:

/contrib/<extension name>/<... full hierarchical path based on extension type ...>//<source|test>/

The latter means that when one looks in the contrib folder they will see a giant list of names with no context beyond that.

[ggreenway]

The reason I suggested copy/paste shared code, is that if it isn't clear who the owner of the shared code is, what happens if one of the extensions using it wants to make a change and the other user doesn't? But I'm fine with the group-consensus to not duplicate the shared code.

[ggreenway]

I think one way to looking at the /contrib/extension-name/full is like github organizations. The extension-name part is like a github org; then there's >=1 extensions underneath it, all owned by the same logical owner.

I think both proposals have merit; I'd be fine with either approach, and as we get more experience with managing this, and if we feel like it, we can re-org it into the other approach if needed. So do whatever is most expedient.

[snowp]

My immediate take is putting extension name on the top level would be pretty messy, especially since I think we'd need the full extension name? We have extensions with the same name for different categories, e.g. envoy.filters.network.rbac and envoy.filters.http.rbac. We might not have this for any of the contrib candidates, but it seems odd to require unique names for extensions when we previously didn't.

[adisuissa]

Changes LGTM, thanks!
Left a couple of suggestions here and in the doc.

[mattklein123]

@htuch @lizan @adisuissa updated per comments

[mattklein123]

Updated, PTAL

[ggreenway]

Docs, policy, directory structure all look good to me. Will defer to other reviewers on the bazel/build-system changes.

[mattklein123]

@lizan @htuch PTAL when you have time, thanks.

[htuch]

Post-merge LGTM, thanks!