Skip to content

Ensure dataplane.Configuration related types don't include v1beta1 types #660

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

Jump to bottom
Closed
kate-osborn opened this issue May 23, 2023 · 0 comments · Fixed by #976
Closed

Ensure dataplane.Configuration related types don't include v1beta1 types #660

kate-osborn opened this issue May 23, 2023 · 0 comments · Fixed by #976
Assignees
@pleshakov
Labels
refined Requirements are refined and the issue is ready to be implemented. size/medium Estimated to be completed within a week tech-debt Short-term pain, long-term benefit
Milestone
v0.6.0

Comments

@kate-osborn
Copy link
Contributor

kate-osborn commented May 23, 2023 •
edited by pleshakov
Loading

Ensure dataplane.Configuration -related types don't include v1beta1 types:

  • This way we don't need to make any validation-related assumptions about them (ex. certain fields cannot be not nil)
  • Also, having dataplane.Configuration separate from Gateway API resources should make it easier to maintain/extend and potentially allow for supporting more routing resource types (outside of Gateway API) in the future.
  • It will simplify unit tests in the configs package.

FIXME in the code -- https://github.com/nginxinc/nginx-gateway-kubernetes/blob/890fddb787ff3560b9b743647a36b649d498ae51/internal/nginx/config/servers.go#L135
@kate-osborn kate-osborn changed the title Placeholder handler.go line 96 Placeholder FIXME "Ensure dataplane.Configuration - related types don't include v1beta1 types May 23, 2023
@kate-osborn kate-osborn changed the title Placeholder FIXME "Ensure dataplane.Configuration - related types don't include v1beta1 types Placeholder FIXME "Ensure dataplane.Configuration - related types don't include v1beta1 types" May 23, 2023
@mpstefan mpstefan added the tech-debt Short-term pain, long-term benefit label Jun 9, 2023
@pleshakov pleshakov changed the title Placeholder FIXME "Ensure dataplane.Configuration - related types don't include v1beta1 types" Ensure dataplane.Configuration related types don't include v1beta1 types Jul 7, 2023
@pleshakov pleshakov removed their assignment Jul 7, 2023
@pleshakov pleshakov mentioned this issue Jul 11, 2023
Merged
6 tasks
@mpstefan mpstefan added backlog Currently unprioritized work. May change with user feedback or as the product progresses. and removed backlog Currently unprioritized work. May change with user feedback or as the product progresses. labels Jul 13, 2023
@mpstefan mpstefan added this to the v0.6.0 milestone Jul 13, 2023
@mpstefan mpstefan added the refined Requirements are refined and the issue is ready to be implemented. label Jul 17, 2023
@mpstefan mpstefan added the size/medium Estimated to be completed within a week label Aug 9, 2023
@pleshakov pleshakov self-assigned this Aug 10, 2023
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this issue Aug 15, 2023
@pleshakov
Remove Gateway API types from Configuration-related types in dataplan…
79bc829 
…e package

Problem:

Configuration-related types in the data plane package include
Gateway API types.

As a result:
- We need to make any validation-related assumptions about them
(ex. certain fields cannot be not nil) in the config package.
- It makes it difficult maintain/extend and potentially allow for
supporting more routing resource types (outside of Gateway API)
in the future.
- It makes unit tests more complicated in the config package

Solution:

- Remove Gateway API types from Configuration-related types.
- Move the types to types.go file.
- Move converters to convert.go

Testing: Unit testing

Closes nginx#660
@pleshakov pleshakov mentioned this issue Aug 15, 2023
Merged
6 tasks
pleshakov added a commit that referenced this issue Aug 18, 2023
@pleshakov
Remove Gateway API types from Configuration-related types in dataplan…
73dc8b0 
…e package (#976)

Problem:

Configuration-related types in the data plane package include
Gateway API types.

As a result:
- We need to make any validation-related assumptions about them
(ex. certain fields cannot be not nil) in the config package.
- It makes it difficult maintain/extend and potentially allow for
supporting more routing resource types (outside of Gateway API)
in the future.
- It makes unit tests more complicated in the config package

Solution:

- Remove Gateway API types from Configuration-related types.
- Move the types to types.go file.
- Move converters to convert.go

Testing: Unit testing

Closes #660
miledxz added a commit to miledxz/nginx-gateway-fabric that referenced this issue Jan 14, 2025
@miledxz
Remove Gateway API types from Configuration-related types in dataplan…
5de4c8f 
…e package (nginx#976)

Problem:

Configuration-related types in the data plane package include
Gateway API types.

As a result:
- We need to make any validation-related assumptions about them
(ex. certain fields cannot be not nil) in the config package.
- It makes it difficult maintain/extend and potentially allow for
supporting more routing resource types (outside of Gateway API)
in the future.
- It makes unit tests more complicated in the config package

Solution:

- Remove Gateway API types from Configuration-related types.
- Move the types to types.go file.
- Move converters to convert.go

Testing: Unit testing

Closes nginx#660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pleshakov pleshakov

Labels
refined Requirements are refined and the issue is ready to be implemented. size/medium Estimated to be completed within a week tech-debt Short-term pain, long-term benefit
Projects
None yet
Milestone
v0.6.0
3 participants
@pleshakov @mpstefan @kate-osborn