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

Object types #777

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Object types #777

merged 2 commits into from
Jul 26, 2024

Conversation

eguzki
Copy link
Contributor

@eguzki eguzki commented Jul 23, 2024

What

  • Policy types
type PolicyType interface {
	GetGVK() schema.GroupVersionKind
	GetInstance() client.Object
	GetList(context.Context, client.Client, ...client.ListOption) ([]Policy, error)
	BackReferenceAnnotationName() string
	DirectReferenceAnnotationName() string
}
  • Type interface for all Gateway API types
type Type interface {
	GetGVK() schema.GroupVersionKind
	GetInstance() client.Object
}
  • Simplified the Policy interface: it should not have anything related to kuadrant.

Useful types to write common controller mappers and controllers that can be used by any policy type.

Work being done as part of #530

This is part of a series of PR's that will end up with a new controller that will only reconcile limitador limits configuration.

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

Attention: Patch coverage is 63.00578% with 64 lines in your changes missing coverage. Please review.

Project coverage is 81.43%. Comparing base (ece13e8) to head (9b65762).
Report is 148 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #777      +/-   ##
==========================================
+ Coverage   80.20%   81.43%   +1.22%     
==========================================
  Files          64       77      +13     
  Lines        4492     6206    +1714     
==========================================
+ Hits         3603     5054    +1451     
- Misses        600      782     +182     
- Partials      289      370      +81     
Flag Coverage Δ
bare-k8s-integration 4.41% <0.00%> (?)
controllers-integration 71.09% <62.42%> (?)
gatewayapi-integration 10.94% <21.96%> (?)
integration ?
istio-integration 55.60% <49.71%> (?)
unit 31.69% <20.80%> (+1.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
api/v1beta1 (u) 71.42% <ø> (ø)
api/v1beta2 (u) 85.35% <80.00%> (-6.08%) ⬇️
pkg/common (u) 88.13% <ø> (-0.70%) ⬇️
pkg/istio (u) 72.50% <ø> (-1.41%) ⬇️
pkg/log (u) 94.73% <ø> (ø)
pkg/reconcilers (u) ∅ <ø> (∅)
pkg/rlptools (u) 83.64% <ø> (+4.19%) ⬆️
controllers (i) 81.62% <83.47%> (+4.81%) ⬆️
Files Coverage Δ
controllers/authpolicy_controller.go 83.94% <100.00%> (+3.23%) ⬆️
controllers/dnspolicy_controller.go 79.01% <100.00%> (-4.47%) ⬇️
controllers/ratelimitpolicy_controller.go 82.52% <100.00%> (+8.52%) ⬆️
controllers/tlspolicy_controller.go 73.63% <100.00%> (+6.96%) ⬆️
pkg/library/kuadrant/gateway_wrapper.go 60.56% <100.00%> (-8.27%) ⬇️
pkg/library/kuadrant/referrer.go 88.23% <100.00%> (+10.45%) ⬆️
pkg/library/mappers/event_mapper.go 100.00% <ø> (ø)
pkg/library/mappers/gateway.go 75.67% <72.72%> (-24.33%) ⬇️
pkg/library/mappers/httproute.go 82.69% <62.50%> (-17.31%) ⬇️
api/v1alpha1/dnspolicy_types.go 72.91% <66.66%> (-8.91%) ⬇️
... and 4 more

... and 29 files with indirect coverage changes

@eguzki eguzki mentioned this pull request Jul 23, 2024
12 tasks
@eguzki eguzki marked this pull request as ready for review July 23, 2024 20:44
@eguzki eguzki requested a review from a team as a code owner July 23, 2024 20:44
@eguzki eguzki requested a review from Boomatang July 23, 2024 20:44
@eguzki
Copy link
Contributor Author

eguzki commented Jul 23, 2024

@Boomatang I would appreciate your 👀 on this PR 🙇‍♂️

Copy link
Contributor

@Boomatang Boomatang left a comment

Choose a reason for hiding this comment

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

I see what is being done and kinda why it is being done. What I am concerned about is the adding of yet more interfaces and structs that we need to be aware of.

My mind is in two places over wide interfaces compared to deep interfaces. What is your view on the abstraction and abstracting to early?

@eguzki
Copy link
Contributor Author

eguzki commented Jul 26, 2024

I see what is being done and kinda why it is being done. What I am concerned about is the adding of yet more interfaces and structs that we need to be aware of.

My mind is in two places over wide interfaces compared to deep interfaces. What is your view on the abstraction and abstracting to early?

I plan to write common (reusable) controller mappers and generic (reusable) controllers that can be used by any policy type. This is kind of preparation work for it. A building block needed for those common controllers.

eguzki added 2 commits July 26, 2024 15:59
Signed-off-by: Eguzki Astiz Lezaun <eastizle@redhat.com>
Signed-off-by: Eguzki Astiz Lezaun <eastizle@redhat.com>
Copy link
Contributor

@Boomatang Boomatang left a comment

Choose a reason for hiding this comment

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

Looks good to me and is a good stepping stone to something better

Copy link
Contributor

@Boomatang Boomatang left a comment

Choose a reason for hiding this comment

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

Comment was to approve, not mark as changes requested

@eguzki eguzki merged commit c1fb9b4 into main Jul 26, 2024
26 of 27 checks passed
@eguzki eguzki deleted the object-types branch July 26, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants