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

k8s gateway extensions: allow customizing translator #9471

Merged
merged 8 commits into from
Jul 24, 2024

Conversation

stevenctl
Copy link
Contributor

Description

API changes

The only API is an internal code API to allow GME to take control of the Gateway translation.

Changed CreatePluginRegistry to GetPluginRegistry. AFAICT there is no need to recreate the registry each time but I can rever tthis if needed.

	// GetPluginRegistry exposes the plugins supported by this implementation.
	GetPluginRegistry() registry.PluginRegistry

Added: GetTranslator to allow conditionally switching the translation logic based on the type of gateway.

	// GetTranslator allows an extnsion to provide custom translation for
	// different gateway classes.
	GetTranslator(*apiv1.Gateway) translator.K8sGwTranslator

Context

See slack conversation here

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [] I have added tests that prove my fix is effective or that my feature works

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label May 9, 2024
Copy link

github-actions bot commented May 9, 2024

Visit the preview URL for this PR (updated for commit 001d07d):

https://gloo-edge--pr9471-stevenctl-allow-cust-hm20ywqs.web.app

(expires Wed, 31 Jul 2024 17:37:27 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

Copy link
Member

@EItanya EItanya left a comment

Choose a reason for hiding this comment

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

Changes LGTM, just want confirmation about the registry creation semantics for portal

projects/gateway2/extensions/extensions.go Outdated Show resolved Hide resolved
EItanya
EItanya previously approved these changes May 23, 2024
@stevenctl stevenctl force-pushed the stevenctl/allow-custom-translator branch 2 times, most recently from 8428f9e to 840cb77 Compare June 11, 2024 19:22
@stevenctl stevenctl closed this Jun 17, 2024
@stevenctl stevenctl reopened this Jun 18, 2024
@stevenctl stevenctl force-pushed the stevenctl/allow-custom-translator branch from 9a25138 to 3af543c Compare July 18, 2024 15:49
@stevenctl stevenctl force-pushed the stevenctl/allow-custom-translator branch from e99ccf5 to 46aecbb Compare July 23, 2024 18:05
jmhbh
jmhbh previously approved these changes Jul 23, 2024
Copy link
Contributor

@jmhbh jmhbh left a comment

Choose a reason for hiding this comment

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

We'll need a PR on the enterprise side to update the implementations of the interface after this PR merges otherwise LGTM

projects/gateway2/extensions/extensions.go Outdated Show resolved Hide resolved
jmhbh
jmhbh previously approved these changes Jul 23, 2024
jmhbh
jmhbh previously approved these changes Jul 23, 2024
@stevenctl stevenctl enabled auto-merge (squash) July 23, 2024 21:21
@stevenctl stevenctl force-pushed the stevenctl/allow-custom-translator branch from c8b4bbe to c41563c Compare July 24, 2024 16:13
@stevenctl stevenctl merged commit 626f869 into main Jul 24, 2024
20 checks passed
@stevenctl stevenctl deleted the stevenctl/allow-custom-translator branch July 24, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants