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.

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

feat: dataplane konnect extension reconciler #714

Merged
merged 14 commits into from
Oct 10, 2024
Merged

feat: dataplane konnect extension reconciler #714

merged 14 commits into from
Oct 10, 2024

Conversation

mlavacca
Copy link
Member

@mlavacca mlavacca commented Oct 8, 2024 •
edited by pmalek
Loading

What this PR does / why we need it:

Which issue this PR fixes

Fixes #203

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch 3 times, most recently from b25bee7 to 74116e3 Compare October 8, 2024 13:03
@mlavacca mlavacca marked this pull request as ready for review October 8, 2024 13:03
@mlavacca mlavacca requested a review from a team as a code owner October 8, 2024 13:03
@mlavacca mlavacca changed the title Mlavacca/dataplane konnect extension reconciler dataplane konnect extension reconciler Oct 8, 2024
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 74116e3 to 653ed0c Compare October 8, 2024 13:11
czeslavo
czeslavo reviewed Oct 8, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
controller/dataplane/konnect_extension.go Outdated Show resolved Hide resolved
controller/dataplane/konnect_extension.go Outdated Show resolved Hide resolved
controller/dataplane/konnect_extension.go Outdated Show resolved Hide resolved
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from f2b31ec to b6c3019 Compare October 9, 2024 09:31
pmalek
pmalek reviewed Oct 9, 2024
View reviewed changes
internal/utils/index/index.go Show resolved Hide resolved
internal/utils/index/index.go Outdated Show resolved Hide resolved
controller/dataplane/dataplanekonnectextension_controller.go Outdated Show resolved Hide resolved
pkg/consts/dataplane.go Show resolved Hide resolved
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 8b3230c to 1843dc6 Compare October 9, 2024 10:29
@mlavacca mlavacca requested review from pmalek and czeslavo October 9, 2024 10:30
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 78970d9 to c4adc17 Compare October 9, 2024 13:25
pmalek
pmalek reviewed Oct 9, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek 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 apart from some nits.

One thing that I believe we're missing here are some tests, specifically a envtest based test or an integration test would definitely make this complete. WDYT?

internal/utils/dataplane/config.go Show resolved Hide resolved
controller/dataplane/konnect_extension.go Show resolved Hide resolved
controller/dataplane/konnect_extension_test.go Outdated Show resolved Hide resolved
controller/dataplane/konnect_extension_test.go Show resolved Hide resolved
controller/dataplane/watch.go Show resolved Hide resolved
controller/dataplane/watch.go Show resolved Hide resolved
@pmalek pmalek added this to the KGO v1.4.x milestone Oct 9, 2024
@mlavacca
Copy link
Member Author

mlavacca commented Oct 9, 2024

Looks good apart from some nits.

One thing that I believe we're missing here are some tests, specifically a envtest based test or an integration test would definitely make this complete. WDYT?

Agreed, I'll add an integration test

@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 91b243e to faac544 Compare October 10, 2024 09:04
@mlavacca mlavacca mentioned this pull request Oct 10, 2024
Open
1 task
@mlavacca
Copy link
Member Author

Looks good apart from some nits.
One thing that I believe we're missing here are some tests, specifically a envtest based test or an integration test would definitely make this complete. WDYT?

Agreed, I'll add an integration test

I've created #726 to track the addition of such an integration test.

@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch 2 times, most recently from fff0529 to 9e722e6 Compare October 10, 2024 09:28
@mlavacca mlavacca requested a review from pmalek October 10, 2024 09:29
pmalek
pmalek reviewed Oct 10, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

Looks pretty much ready to me. I've left 1 comment with a proposal to change the condition message and 2 nits.

controller/dataplane/controller_utils.go Outdated Show resolved Hide resolved
controller/dataplane/controller_utils.go Outdated Show resolved Hide resolved
controller/dataplane/dataplanekonnectextension_controller.go Show resolved Hide resolved
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 78ba569 to fa3f744 Compare October 10, 2024 12:43
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from fa3f744 to 9b78974 Compare October 10, 2024 12:52
@mlavacca mlavacca mentioned this pull request Oct 10, 2024
Open
1 task
@mlavacca mlavacca enabled auto-merge (squash) October 10, 2024 12:56
pmalek
pmalek reviewed Oct 10, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

1 thing I've just realized is that we're missing RBAC permissions for accessing DataPlaneKonnectExtensions (soon to be renamed to KonnectExtensions).

Do you mind adding kubebuilder RBAC markers to https://github.com/Kong/gateway-operator/blob/6b9a627106115acc27048f5fe57b0f063fbb9449/controller/dataplane/controller_rbac.go, adding said markers also for DataPlaneKonnectExtensionReconciler in controller/dataplane/dataplanekonnectextension_controller_rbac.go
and regenerating the manifests?

mlavacca and others added 14 commits October 10, 2024 15:05
@mlavacca
feat: DataPlaneKonnectExtension customization
b3cf082 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
unit test and API field renamed
0173572 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca @czeslavo
Apply suggestions from code review
adb5313 
Co-authored-by: Grzegorz Burzyński <[email protected]>
@mlavacca
don't allow cross-namespace refs
6cace45 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
gathered util with consts
1cb2b76 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
address comments
66e306c 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
refactor code
9d26152 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
fix lint
0dfdb76 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
multiple params to struct of params
b3e2461 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
address review comments
375fa7d 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
feat: add ResolvedRefs condition to DataPlane
aaa3cc4 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
chore
57e0c9d 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
customize the error message
b97bd09 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca
rbacs updated
c46913e 
Signed-off-by: Mattia Lavacca <[email protected]>
@mlavacca mlavacca force-pushed the mlavacca/dataplane-konnect-extension-reconciler branch from 1e179ec to c46913e Compare October 10, 2024 13:05
@mlavacca mlavacca requested a review from pmalek October 10, 2024 13:06
@mlavacca mlavacca changed the title dataplane konnect extension reconciler feat: dataplane konnect extension reconciler Oct 10, 2024
@mlavacca mlavacca merged commit 0780829 into main Oct 10, 2024
21 checks passed
@mlavacca mlavacca deleted the mlavacca/dataplane-konnect-extension-reconciler branch October 10, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@czeslavo czeslavo czeslavo approved these changes

@pmalek pmalek pmalek approved these changes

Assignees

@mlavacca mlavacca

Labels
area/konnect area/konnect-DPs
Projects
None yet
Milestone
KGO v1.4.x
Development

Successfully merging this pull request may close these issues.

Native support for Konnect integration in DataPlane
3 participants
@mlavacca @pmalek @czeslavo