Skip to content

fix: preserve plugins with empty config using targeted approach#3277

Open
DSingh0304 wants to merge 4 commits intoapache:masterfrom
DSingh0304:fix/plugin-config-targeted
Open

fix: preserve plugins with empty config using targeted approach#3277
DSingh0304 wants to merge 4 commits intoapache:masterfrom
DSingh0304:fix/plugin-config-targeted

Conversation

@DSingh0304
Copy link
Contributor

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Fields with empty object values ({}) were being silently dropped when saving. This affects:

  • Plugins that don't require configuration (e.g., key-auth, prometheus, echo)
  • Upstream discovery_args when using Service Discovery

Root Cause: The fast-clean library used in src/utils/producer.ts has emptyObjectsCleaner: true by default, which removes empty objects before sending data to the server.

Fix: Set emptyObjectsCleaner: false in the deepCleanEmptyKeys function to preserve empty objects.

Related issues

fix #3269
fix #3270

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

This introduces a targeted fix to preserve empty plugin configurations (like key-auth)
during the data cleaning process.

- Adds producePreservePlugins to mark empty plugin configs before cleaning
- Adds produceRemovePreserveMarkers to restore them after cleaning
- Keeps default cleaning behavior for other fields to prevent validation errors
Copy link
Member

Choose a reason for hiding this comment

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

Hi, pls add a test for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes i will add the test in some time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey I have added the test...

Copy link
Member

Choose a reason for hiding this comment

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

sorry for too late.

Copy link
Contributor

@Baoyuantop Baoyuantop left a comment

Choose a reason for hiding this comment

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

The PR description also mentions fixing #3270

We also need to write tests to verify this issue.

Comment on lines 34 to 47

// Create a route via API with key-auth plugin having empty configuration
const response = await postRouteReq(e2eReq, {
name: routeNameWithEmptyPlugin,
uri: routeUri,
plugins: {
'key-auth': {},
},
upstream: {
nodes: [{ host: 'httpbin.org', port: 80, weight: 1 }],
type: 'roundrobin',
},
});

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is also part of the testing path and should be created within the testing process rather than through the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay I ll look into it..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Baoyuantop I have made the changes...

@DSingh0304
Copy link
Contributor Author

The PR description also mentions fixing #3270

We also need to write tests to verify this issue.

I will add it soon...

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.

Discovery Args are droped when edit upstream Plugin with empty json config is dropped when add or edit Route

3 participants