Feat: Extension server policy propagation to PostTranslateModify

[raviverma-ai]

What type of PR is this?

• "feat(translator): Add extension server policies to PostTranslateContext so that extension server can take actions accordingly."

What this PR does / why we need it:
Add custom extension server policies to PostTranslateContext and eventually pass to PostTranslateModify() method so that extension server can take actions accordingly based on CRDs.

Release Notes: Yes

[codecov]

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.49%. Comparing base (e07d1ec) to head (48d054b).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/extension/registry/xds_hook.go	80.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5914      +/-   ##
==========================================
- Coverage   70.52%   70.49%   -0.04%     
==========================================
  Files         220      220              
  Lines       36621    36639      +18     
==========================================
  Hits        25828    25828              
- Misses       9268     9285      +17     
- Partials     1525     1526       +1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[guydc]

Can we avoid saving the extension policies twice per XDS? they are saved for both the entire XDS and the IR Listener now.

[arkodg]

hey @raviverma-ai can you share your use case, curious why the other hooks aren't applicable

[guydc]

cc @nareddyt

[muwaqar]

this seems to be doing what I suggested in #5561, but it would be good to know what the specific use-case for the PR author is (as arko said here: #5914 (comment))

[raviverma-ai]

hey @raviverma-ai can you share your use case, curious why the other hooks aren't applicable

@arkodg, @muwaqar

This enables users to perform actions which needs modifications to clusters (other hooks are not proving options to modify clusters). In our particular use case, we would like to enable sds based mtls certificate issuance and rotation instead of manually updating certificates. This requires clusters to have UpstreamTlsContext pointing to sds server and get certificates automatically. In our case, we are creating native integration with Istio and to avoid sidecar injection on EG.

[arkodg]

thanks @raviverma-ai, the use case makes sense, do you need access to Extension Resources for this case ?

[raviverma-ai]

@arkodg, Yes, it would give more control to user to take actions on. For instance, we are using extension server policy CRD to enable this feature bound to specific gateway. i.e. If user wants to enable sds based mtls for specific gateway and not for all gateways. Furthermore, we are planning to provide even more control in future like user would be able to control this behaviour on httproute or backend/service basis (still under discussion for feasibility and use case).

[guydc]

hi @raviverma-ai. Thoughts on this comment from earlier?

Can we avoid saving the extension policies twice per XDS? they are saved for both the entire XDS and the IR Listener now.

[guydc]

cc @liorokman

[raviverma-ai]

hi @raviverma-ai. Thoughts on this comment from earlier?

Can we avoid saving the extension policies twice per XDS? they are saved for both the entire XDS and the IR Listener now.

@guydc thanks for reminding. Updated logic to store the pointer to original policies instead of creating another copy.

[guydc]

@guydc thanks for reminding. Updated logic to store the pointer to original policies instead of creating another copy.

I think that this should be reflected in the test files as well. Can you regenerate the test output files with make go.testdata.complete so that we can see the impact?

Also, please add a few test cases for this new flow to internal/xds/testdata/extension-xds-ir and extend testingExtensionServer to have some logic to cover:

• Creation of new secrets/clusters based on extension resources
• Emitting errors and handling them in the extension manager

[raviverma-ai]

@guydc thanks for reminding. Updated logic to store the pointer to original policies instead of creating another copy.

I think that this should be reflected in the test files as well. Can you regenerate the test output files with make go.testdata.complete so that we can see the impact?

Also, please add a few test cases for this new flow to internal/xds/testdata/extension-xds-ir and extend testingExtensionServer to have some logic to cover:

• Creation of new secrets/clusters based on extension resources
• Emitting errors and handling them in the extension manager

@guydc added/updated test cases as requested.
p.s. changes with 'make go.testdata.complete' were submitted as well.

[guydc]

@raviverma-ai - one question on my end. Right now, for gateway-attached policies, we save the relevant policy for the appropriate listener in the gateway and provide it as context in that listener's scope.

Here, we seem to store all policies on the gateway level and send them as context for the post-translate hook. As a user, does that meet your requirements? Do you not need to know which specific clusters (originating from specific listeners) should be affected by the policy?

[raviverma-ai]

@guydc As I had mentioned earlier as well. For this or any similar use case, where user only needs to know which Gateway, that specific Extension Server Policy is applicable to, i.e. all dynamic cluster can be modified/reviewed or even specific ones as well, those information can be specified in Custom Extension Server Policy defined by user. For now, we will be using this to enable mtls via sds server on all backend communications. However, there is plan for future to even further fine tune and target specific route/service etc.
Hope, I was able to answer your question.

[arkodg]

can we use the same naming used in other places like L264

	// ExtensionRefs holds unstructured resources that were introduced by an extension policy
	ExtensionRefs []*UnstructuredRef `json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"`

[arkodg]

LGTM thanks
added one non blocker comment

[guydc]

@raviverma-ai

can we use the same naming used in other places like L264
// ExtensionRefs holds unstructured resources that were introduced by an extension policy
ExtensionRefs []*UnstructuredRef json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"

Can you fix this in a follow-up PR please?

[raviverma-ai]

@raviverma-ai

can we use the same naming used in other places like L264
// ExtensionRefs holds unstructured resources that were introduced by an extension policy
ExtensionRefs []*UnstructuredRef json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"

Can you fix this in a follow-up PR please?

Yes, sure.