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

How to Organize API Types #809

Closed
danehans opened this issue Dec 14, 2022 · 12 comments · Fixed by #1921
Closed

How to Organize API Types #809

danehans opened this issue Dec 14, 2022 · 12 comments · Fixed by #1921
Assignees
Labels
area/api API-related issues
Milestone

Comments

@danehans
Copy link
Contributor

danehans commented Dec 14, 2022

Currently, EG API types reside in 2 locations:

  • Envoy Gateway and Envoy Proxy configuration: api/config/v1alpha1.
  • Extended functionality: api/v1alpha1.

This makes imports a bit confusing:

import (
	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
	egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
)

It's unclear what API types are being imported from github.com/envoyproxy/gateway/api/v1alpha1.

@danehans danehans added the kind/decision A record of a decision made by the community. label Dec 14, 2022
@danehans
Copy link
Contributor Author

From the 5/13/22 EG community call, @youngnick proposes:

I recommend keeping all the types in the version directory (eg v1alpha1), with validation, helper, or utility code in subdirectories.

@danehans danehans added this to the 0.3.0-rc.1 milestone Dec 14, 2022
@danehans
Copy link
Contributor Author

My recommendation slightly differs from the above. I suggest having separate config and extension API groups. Helper functions, validation, etc. will live within each group for the associated types. This allows versioning the extension types separately from the config types. For example, the EnvoyProxy type can graduate faster than extension API types since they may be more widely used. Additionally, it simplifies transitioning the extension APIs into a separate repo.

Thoughts @youngnick

@zirain
Copy link
Contributor

zirain commented Dec 19, 2022

+1 to egv1 "github.com/envoyproxy/gateway/api/v1alpha1" or egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1"

@youngnick
Copy link
Contributor

@danehans, I think that's fine, although I suggest standardizing the import names to make reading the code easier.

@danehans
Copy link
Contributor Author

@arkodg @AliceProxy @Xunzhuo @skriss please comment if you have issue with the approach in #809 (comment).

@arkodg
Copy link
Contributor

arkodg commented Dec 19, 2022

@danehans i think theres a typo in the issue description - Extended functionality: api/v1alpha1
as @zirain suggested I prefer all EG APIs to live under api/v1alpha1 under the same group for now so there are lesser groups to revision

@danehans
Copy link
Contributor Author

i think theres a typo in the issue description

@arkodg thanks for catching this. I updated the description.

I prefer all EG APIs to live under api/v1alpha1 under the same group for now so there are lesser groups to revision

If our long-term intention is for extensions to live outside of the EG repo, they'll be revisioned separately.

@danehans danehans modified the milestones: 0.3.0-rc.1, 0.4.0 Jan 12, 2023
@danehans danehans added help wanted Extra attention is needed area/api API-related issues labels Jan 12, 2023
@Alice-Lilith Alice-Lilith modified the milestones: 0.4.0, 0.5.0-rc1 Apr 24, 2023
@arkodg
Copy link
Contributor

arkodg commented Jul 11, 2023

this was discussed in the community meeting today
@AliceProxy and I are in agreement to move the Group from config.gateway.envoyproxy.io to gateway.envoyproxy.io
https://github.com/envoyproxy/gateway/blob/main/api/config/v1alpha1/groupversion_info.go#L15 in v0.5.0.

This will be a breaking change for users using the EnvoyProxy resource today.

@envoyproxy/gateway-maintainers, would be great if you can 👍 👎 ?

@kflynn
Copy link
Contributor

kflynn commented Jul 18, 2023

Until we're at 1.0.0, we should not be afraid of breaking changes. Go for it.

@Alice-Lilith
Copy link
Member

Alice-Lilith commented Jul 18, 2023

Until we're at 1.0.0, we should not be afraid of breaking changes. Go for it.

+1 for callpsing everything into gateway.envoyproxy.io and making the breaking change without a deprecation policy or any kind of "migration". Nobody should be running this in production right now.

@zirain
Copy link
Contributor

zirain commented Jul 19, 2023

@envoyproxy/gateway-steering-committee @envoyproxy/nighthawk-maintainers we need a certain policy for support and deprecation.

@arkodg
Copy link
Contributor

arkodg commented Jul 19, 2023

@zirain deprecation policy is being tracked with #714

@arkodg arkodg modified the milestones: 0.5.0-rc1, 0.6.0-rc1 Jul 26, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Oct 3, 2023
* this keeps all CRDs/APIs under the gateway.envoyproxy.io group

* this is a breaking change, that needs to be highlighted
  in the release notes. Undesirable but acceptable since these
  APIs are experimental (alpha) and are not yet stable

Fixes: envoyproxy#809

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Oct 3, 2023
* this keeps all CRDs/APIs under the gateway.envoyproxy.io group

* this is a breaking change, that needs to be highlighted
  in the release notes. Undesirable but acceptable since these
  APIs are experimental (alpha) and are not yet stable

Fixes: envoyproxy#809

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg self-assigned this Oct 3, 2023
@arkodg arkodg removed help wanted Extra attention is needed kind/decision A record of a decision made by the community. labels Oct 3, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Oct 4, 2023
* this keeps all CRDs/APIs under the gateway.envoyproxy.io group

* this is a breaking change, that needs to be highlighted
  in the release notes. Undesirable but acceptable since these
  APIs are experimental (alpha) and are not yet stable

Fixes: envoyproxy#809

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain pushed a commit that referenced this issue Oct 5, 2023
* move EnvoyProxy API to gateway.envoyproxy.io

* this keeps all CRDs/APIs under the gateway.envoyproxy.io group

* this is a breaking change, that needs to be highlighted
  in the release notes. Undesirable but acceptable since these
  APIs are experimental (alpha) and are not yet stable

Fixes: #809

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix config

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix rbac

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants