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

add feature online plugins filter flag #1458

Merged
merged 7 commits into from
Dec 6, 2024

Conversation

AntoineJac
Copy link
Collaborator

Add a new flag to only validate plugins online

@mheap
Copy link
Member

mheap commented Nov 28, 2024

Deck is adding lots of these "only X" or "skip Y" flags. Can we make this more generic, so that they can pass something like:

--validate-only plugins,consumers?

@codecov-commenter
Copy link

codecov-commenter commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 60 lines in your changes missing coverage. Please review.

Project coverage is 27.59%. Comparing base (4da4246) to head (852694d).

Files with missing lines Patch % Lines
validate/validate.go 0.00% 37 Missing ⚠️
cmd/gateway_validate.go 0.00% 23 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1458      +/-   ##
==========================================
+ Coverage   27.55%   27.59%   +0.04%     
==========================================
  Files          61       61              
  Lines        6294     6283      -11     
==========================================
  Hits         1734     1734              
+ Misses       4432     4421      -11     
  Partials      128      128              

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

@AntoineJac
Copy link
Collaborator Author

AntoineJac commented Nov 28, 2024

@mheap sure I was actually thinking about something similar first. Let me refactor this PR

For info it really does have a nice impact on performance, here a test with a small file:
Screenshot 2024-11-28 at 15 42 48

Performance are almost 40% to 50% better

@AntoineJac
Copy link
Collaborator Author

AntoineJac commented Nov 28, 2024

@mheap can you confirm it looks ok or do you want me to edit the flag name? I will then ask for review for this PR.
I have also slightly refactor to simplify the code and use a EntityMap global var instead of running test by test

Thanks

FlagName: --online-entities-list

Entities should be part of the EntitiesMap: RBACEndpointPermissions, RBACRoles, Plugins, Services, ACLGroups, BasicAuths, CACertificates, Certificates, Consumers, Documents, HMACAuths, JWTAuths, KeyAuths, Oauth2Creds, Routes, SNIs, Targets, Upstreams, FilterChains, Vaults.

mheap
mheap previously requested changes Dec 2, 2024
cmd/gateway_validate.go Outdated Show resolved Hide resolved
@mheap mheap self-requested a review December 2, 2024 13:34
fix reflection

add test

fix lint

fix lint

add valid entities

fux lint

Updated go-apiops (#1452)

* chore: updated go-apiops to v0.1.40

* chore: release prep for v1.41.4

fux lint

fix lint

fix lint
@AntoineJac AntoineJac force-pushed the feat/online-plugins-validate-field branch from beb2319 to 6fe17ee Compare December 2, 2024 16:03
@AntoineJac
Copy link
Collaborator Author

Any update on this? Thanks

@mheap
Copy link
Member

mheap commented Dec 3, 2024

@Prashansa-K Please review

@AntoineJac AntoineJac force-pushed the feat/online-plugins-validate-field branch from c74b7cf to 1358190 Compare December 4, 2024 16:02
cmd/gateway_validate.go Outdated Show resolved Hide resolved
cmd/gateway_validate.go Show resolved Hide resolved
cmd/gateway_validate.go Outdated Show resolved Hide resolved
tests/integration/validate_test.go Outdated Show resolved Hide resolved
tests/integration/validate_test.go Outdated Show resolved Hide resolved
@AntoineJac AntoineJac force-pushed the feat/online-plugins-validate-field branch from 189e17d to 3669938 Compare December 5, 2024 10:33
@AntoineJac AntoineJac force-pushed the feat/online-plugins-validate-field branch from e3d0680 to 2273715 Compare December 5, 2024 10:41
@AntoineJac AntoineJac force-pushed the feat/online-plugins-validate-field branch from 589b1cf to 4862fa7 Compare December 5, 2024 10:43
@AntoineJac AntoineJac removed the request for review from GGabriele December 6, 2024 11:04
@AntoineJac AntoineJac enabled auto-merge (squash) December 6, 2024 11:05
@AntoineJac AntoineJac disabled auto-merge December 6, 2024 11:05
@AntoineJac AntoineJac merged commit 4c52f38 into main Dec 6, 2024
19 checks passed
@AntoineJac AntoineJac deleted the feat/online-plugins-validate-field branch December 6, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants