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

feat(custom_entity): add KCE controller #6055

Merged
merged 8 commits into from
Jun 5, 2024

Conversation

randmonkey
Copy link
Contributor

@randmonkey randmonkey commented May 20, 2024

What this PR does / why we need it:

The initial (and largest) part of KongCustomEntity controller.

Which issue this PR fixes:

Major part of #5976

Special notes for your reviewer:

It invokes a procedure to get referred entities of plugins to fill IDs of the entities to the "foreign" fields of custom entities. It is finished in getServiceIDFromPluginRels method. I think we can change the getPluginRels method to store pointers to the entities instead of identifiers, because with the entities itself we can do more operations on it.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey changed the title WIP add KCE controller [WIP] feat(custom_entity): add KCE controller May 20, 2024
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 43.00792% with 216 lines in your changes missing coverage. Please review.

Project coverage is 74.3%. Comparing base (763d7d1) to head (8a704cd).
Report is 40 commits behind head on main.

Files Patch % Lines
...trollers/configuration/zz_generated_controllers.go 0.0% 92 Missing ⚠️
internal/dataplane/kongstate/kongstate.go 56.8% 63 Missing and 10 partials ⚠️
internal/dataplane/sendconfig/inmemory.go 0.0% 9 Missing and 1 partial ⚠️
internal/store/store.go 52.3% 10 Missing ⚠️
internal/dataplane/translator/translator.go 18.1% 8 Missing and 1 partial ⚠️
internal/dataplane/deckgen/deckgen.go 16.6% 4 Missing and 1 partial ⚠️
internal/manager/schema_service.go 37.5% 5 Missing ⚠️
internal/store/fake_store.go 42.8% 3 Missing and 1 partial ⚠️
internal/controllers/utils/utils.go 0.0% 2 Missing and 1 partial ⚠️
internal/dataplane/kong_client.go 40.0% 2 Missing and 1 partial ⚠️
... and 1 more
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #6055     +/-   ##
=======================================
- Coverage   74.6%   74.3%   -0.3%     
=======================================
  Files        194     198      +4     
  Lines      18861   19743    +882     
=======================================
+ Hits       14072   14677    +605     
- Misses      3814    4061    +247     
- Partials     975    1005     +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch 3 times, most recently from 3a1f792 to 663d12d Compare May 30, 2024 08:39
@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch 3 times, most recently from 9dc3bfa to e1acce3 Compare May 30, 2024 10:04
@randmonkey randmonkey closed this May 31, 2024
@randmonkey randmonkey reopened this May 31, 2024
@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch from e1acce3 to d91cdc0 Compare May 31, 2024 10:29
@randmonkey randmonkey changed the title [WIP] feat(custom_entity): add KCE controller feat(custom_entity): add KCE controller May 31, 2024
@randmonkey randmonkey marked this pull request as ready for review May 31, 2024 10:34
@randmonkey randmonkey requested a review from a team as a code owner May 31, 2024 10:34
@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch from d91cdc0 to 76e0289 Compare May 31, 2024 10:36
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Show resolved Hide resolved
internal/dataplane/kongstate/kongstate_test.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate_test.go Outdated Show resolved Hide resolved
docs/cli-arguments.md Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Show resolved Hide resolved
internal/manager/run.go Outdated Show resolved Hide resolved
internal/dataplane/kong_client.go Outdated Show resolved Hide resolved
internal/dataplane/deckgen/deckgen.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/customentity.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/inmemory.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/inmemory.go Outdated Show resolved Hide resolved
internal/store/fake_store.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
internal/controllers/utils/utils.go Outdated Show resolved Hide resolved
internal/dataplane/fallback/graph_dependencies.go Outdated Show resolved Hide resolved
internal/manager/config.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/inmemory.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate_test.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch 2 times, most recently from 00a8f9b to 661c6ab Compare June 4, 2024 08:21
@randmonkey randmonkey requested a review from pmalek June 4, 2024 08:22
@randmonkey randmonkey force-pushed the feat/custom_entity_controller branch from 661c6ab to 7f17bca Compare June 4, 2024 09:04
internal/manager/run.go Outdated Show resolved Hide resolved
internal/manager/run.go Outdated Show resolved Hide resolved
internal/dataplane/translator/translator_test.go Outdated Show resolved Hide resolved
internal/dataplane/translator/translator.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Outdated Show resolved Hide resolved
internal/controllers/utils/utils.go Show resolved Hide resolved
internal/dataplane/kongstate/kongstate.go Show resolved Hide resolved
@randmonkey randmonkey requested a review from pmalek June 5, 2024 05:51
internal/dataplane/deckgen/deckgen.go Outdated Show resolved Hide resolved
internal/manager/run.go Outdated Show resolved Hide resolved
internal/manager/run.go Outdated Show resolved Hide resolved
internal/manager/run.go Show resolved Hide resolved
internal/manager/featuregates/feature_gates_test.go Outdated Show resolved Hide resolved
internal/dataplane/kongstate/customentity.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/strategy.go Outdated Show resolved Hide resolved
@randmonkey randmonkey requested a review from pmalek June 5, 2024 10:54
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.

Minor typos and we're close to merging this.

Just for clarification about fakes vs mocks vs stubs which I believe is already incorrectly used across the codebase:

We're referring to fakes in our codebase to entities that are service a purpose that is reserved for stubs.

Fakes are meant to be implementations that perform some sort of a shortcut (e.g. use memory backed storage instead of DB). Stubs are meant to

provide canned answers to calls made during the test, usually not responding at all to anything outside of what’s programmed.

internal/dataplane/sendconfig/strategy.go Outdated Show resolved Hide resolved
internal/dataplane/translator/translator.go Outdated Show resolved Hide resolved
internal/dataplane/translator/translator.go Outdated Show resolved Hide resolved
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.

One last thing which I believe is important: can we add at least 1 example manifest to https://github.com/Kong/kubernetes-ingress-controller/tree/9dfe2123b277e8ba418152ff1536c9e696786a7f/examples that will use the custom entity CRD?

@randmonkey
Copy link
Contributor Author

One last thing which I believe is important: can we add at least 1 example manifest to https://github.com/Kong/kubernetes-ingress-controller/tree/9dfe2123b277e8ba418152ff1536c9e696786a7f/examples that will use the custom entity CRD?

I would include this in PR for integration tests.

@randmonkey randmonkey requested a review from pmalek June 5, 2024 13:30
@randmonkey randmonkey merged commit 83240a1 into main Jun 5, 2024
40 checks passed
@randmonkey randmonkey deleted the feat/custom_entity_controller branch June 5, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants