Implement agentgateway emitter

[markuskobler]

What type of PR is this?
/kind feature

What this PR does / why we need it:
This adds initial support for Agentgateway. Note that Agentgateway has undergone significant changes recently as it gears up for a 1.0.0 release. Critical to this feature is the migration of the controller out of kgateway and into github.com/agentgateway/agentgateway.

Which issue(s) this PR fixes:
Fixes #364

Does this PR introduce a user-facing change?:

Initial support for Agentgateway resources

[k8s-ci-robot]

Hi @markuskobler. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[Stevenjin8]

/ok-to-test

[markuskobler]

@Stevenjin8 apologies I missed I checked in the go.sum. Can you re-run the tests?

[markuskobler]

/retest

[markuskobler]

/test pull-ingress2gateway-test

[markuskobler]

I'm not clear on the cause of the verify error. Running make verify locally returns

hack/verify-all.sh -v
Skipping hack/../hack/verify-all.sh
Verified 0 file headers match boilerplate
SUCCESS: hack/../hack/verify-boilerplate.sh
0 issues.
SUCCESS: hack/../hack/verify-golint.sh

[Stevenjin8]

/retest

[Stevenjin8]

@markuskobler also, I'm planning on mergeing #366 (its really annoying to rebase) before this PR, so you might want to get a head start on the rebase.

[markuskobler]

@markuskobler also, I'm planning on mergeing #366 (its really annoying to rebase) before this PR, so you might want to get a head start on the rebase.

Thanks for the heads up will try to look at this later today

[Stevenjin8]

@markuskobler We are looking to do a 1.0 release next week btw

[markuskobler]

@markuskobler We are looking to do a 1.0 release next week btw

@Stevenjin8 sounds good. We are also looking a agw 1.0 release by the end of this week now that the controller is fully split from kgateway.

[Stevenjin8]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markuskobler, Stevenjin8

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Stevenjin8]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[johananl]

Do we need this helper for any tests? If so - I'm not seeing any new test cases in this PR. If not, I wonder if we should add a helper we aren't using.

[markuskobler]

Good question. Given that the agentgateway controller was split out from kgateway I initially just duplicated the kgateway e2e test.

[johananl]

Might be an opportunity to implement the first emitter test - see category 4 in https://github.com/kubernetes-sigs/ingress2gateway/blob/main/e2e/README.md.

WDYT?

[Stevenjin8]

+1 on adding a quick test. Just a sanity check that it works to help me sleep at night

[markuskobler]

Sorry I thought I had correctly wired up the tests. It should be used now

[johananl]

This adds a smoke test for agentgateway as an implementation. This does not test any emitter features. Given that this PR is titled "Implement agentgateway emitter" I just wanted to make sure this is what you intended.

[markuskobler]

We are planning to add more functionality now that agentgateway has released v1.0 and is no longer part of kgateway, which is what a previous PR was attempting to add agentgateway emitter support against.

The main reason I avoided adding anything beyond logging a warning for unsupported functionality was the number of changes the current go.mod would pull in. I hope to get the controller go.mod split from the api later this week or next.

I can make this a much bigger change if you wanted however?

[johananl]

I don't have the full context here and have no strong opinions. Just wanted to ensure you're aware what the test you've added does and doesn't do 🙂 I'll leave it up to you.

[johananl]

FWIW I think it's a good idea to have agentgateway in the implementation smoke tests.

[Stevenjin8]

I will merge this by lazy consensus before the 1.0 release if there are no more comments.

[johananl]

Curious: Why two separate lines?

[markuskobler]

My editor adds a newline at end of file. Happy to minimize the diff if you'd prefer.

[Stevenjin8]

Tell me you use vim without telling me you use vim. I like the new line

[markuskobler]

I will merge this by lazy consensus before the 1.0 release if there are no more comments.

I'm hoping to get a PR in adding support for AgentGateway regex path rewrites, which should make the emitter slightly less trivial. I've just been swamped with work this last week.

[Stevenjin8]

@markuskobler if ou look at the path rewrite code, I've set it so it's pretty easy for you to add regex rewrites without changing provider code

[markuskobler]

@markuskobler if ou look at the path rewrite code, I've set it so it's pretty easy for you to add regex rewrites without changing provider code

@Stevenjin8 I'm assuming your talking about the emitterir.PathRewrite?

ingress2gateway/pkg/i2gw/emitter_intermediate/intermediate_representation.go

Line 141 in 4a7a91e

RegexCaptureGroupReferences bool

My plan was to convert the regex paths to agentgateway specific CEL equivalents. But happy to take feedback if you think there is a better way of implementing this?

[Stevenjin8]

/lgtm