Use <proto>-<port> for naming service and container ports by arkodg · Pull Request #3130 · envoyproxy/gateway

arkodg · 2024-04-08T13:13:37Z

Takes inspiration from #2973 to name port, not off the listener but off the port-proto ensuring that patch (during updates) also works

Fixes #3130 (comment) by adding a stable name

zirain · 2024-04-08T13:15:02Z

internal/cmd/egctl/testdata/translate/out/quickstart.all.yaml

like this naming rule

arkodg · 2024-04-08T13:27:02Z

cc @cnvergence @ardikabs

cnvergence · 2024-04-08T15:30:45Z

that was fast!

owenhaynes · 2024-04-09T11:58:45Z

This is very much needed as it will fix the constant redeploying

arkodg · 2024-04-09T12:12:07Z

@owenhaynes can you elaborate, the constant redeploying should have been fixed with #2995

owenhaynes · 2024-04-09T12:41:04Z

@arkodg

In my example we have 4+ listeners attached to a merge host gateway using the listeners name "https". Every time the gateway checks to see if the deployment is in-sync the container port name is different as its randomly getting a different listener name, this causes the deployment to get updated as the specs do not match.

This is tested with image envoyproxy/gateway-dev:5a15902 which is the tag which contains the backports.

I assume the Service is also getting patched every time with a new port name, but I don't think that would cause any issues

arkodg · 2024-04-09T12:43:53Z

ah that randomness is coming from

@arkodg

In my example we have 4+ listeners attached to a merge host gateway using the listeners name "https". Every time the gateway checks to see if the deployment is in-sync the container port name is different as its randomly getting a different listener name, this causes the deployment to get updated as the specs do not match.

This is tested with image envoyproxy/gateway-dev:5a15902 which is the tag which contains the backports.

I assume the Service is also getting patched every time with a new port name, but I don't think that would cause any issues

ah thanks, this PR should help add stability there

cnvergence · 2024-04-09T12:51:53Z

yes, in this case this PR should do the job, as this is the next step for #3061

owenhaynes · 2024-04-11T11:56:40Z

Here is the EnvoyProxy patchs for people to use if they are having the same problem until this resolved.

    envoyProxy:
      provider:
        kubernetes:
          envoyService:
            patch:
              value:
                spec:
                  ports:
                  - name: https
                    port: 443
                    protocol: TCP
                    targetPort: 10443
          envoyDeployment:
            patch:
              value:
                spec:
                  template:
                    spec:
                      containers:
                      - name: envoy
                        ports:    
                        - containerPort: 10443
                          name: https
                          protocol: TCP

Takes inspiration from envoyproxy#2973 to name port, not off the listener but off the port-proto ensuring that patch (during updates) also works Fixes: envoyproxy#3111 Signed-off-by: Arko Dasgupta <arko@tetrate.io>

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

internal/gatewayapi/listener.go

cnvergence

lgtm

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

zirain · 2024-04-15T23:41:17Z

/retest

zirain · 2024-04-16T02:53:55Z

/retest

shawnh2

seems we should also change the e2e test case for this one

gateway/test/e2e/tests/gateway_infra_resource.go

Line 132 in e537033

hashedPortName := utils.GetHashedName(newListenerTCPName, 6)

according to the failure of e2e test https://github.com/envoyproxy/gateway/actions/runs/8688528949/job/23858439302?pr=3130#step:6:2966

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

arkodg · 2024-04-16T06:20:02Z

gateway/test/e2e/tests/gateway_infra_resource.go

thanks for spotting this !

zirain · 2024-04-16T06:29:49Z

lint fails

Error: : # github.com/envoyproxy/gateway/test/e2e/tests
Error: test/e2e/tests/gateway_infra_resource.go:25:2: "github.com/envoyproxy/gateway/internal/utils" imported and not used (typecheck)
Error: could not import github.com/envoyproxy/gateway/test/e2e/tests (-: # github.com/envoyproxy/gateway/test/e2e/tests
Error: test/e2e/tests/gateway_infra_resource.go:25:2: "github.com/envoyproxy/gateway/internal/utils" imported and not used) (typecheck)
Error: could not import github.com/envoyproxy/gateway/test/e2e/tests (-: # github.com/envoyproxy/gateway/test/e2e/tests
Error: test/e2e/tests/gateway_infra_resource.go:25:2: "github.com/envoyproxy/gateway/internal/utils" imported and not used) (typecheck)

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

…#3130) * Use <proto>-<port> for naming service and container ports Takes inspiration from envoyproxy#2973 to name port, not off the listener but off the port-proto ensuring that patch (during updates) also works Fixes: envoyproxy#3111 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * testdata Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix test Signed-off-by: Arko Dasgupta <arko@tetrate.io> * move to helper pkg Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix e2e Signed-off-by: Arko Dasgupta <arko@tetrate.io> * lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix e2e Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit c41247b) Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* Use <proto>-<port> for naming service and container ports (#3130) * Use <proto>-<port> for naming service and container ports Takes inspiration from #2973 to name port, not off the listener but off the port-proto ensuring that patch (during updates) also works Fixes: #3111 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * testdata Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix test Signed-off-by: Arko Dasgupta <arko@tetrate.io> * move to helper pkg Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix e2e Signed-off-by: Arko Dasgupta <arko@tetrate.io> * lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix e2e Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit c41247b) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * bug: Tests are failing due to an expired certificate in one of the translator tests (#3476) Replaced a certificate in the test that had expired. The old certificate expired May 24 2024: Certificate: Data: Version: 1 (0x0) Serial Number: ca:7c:5c:b7:25:5d:bb:f9 Signature Algorithm: ecdsa-with-SHA256 Issuer: CN=test.example.com Validity Not Before: May 25 14:10:42 2023 GMT Not After : May 24 14:10:42 2024 GMT Subject: CN=test.example.com Subject Public Key Info: Public Key Algorithm: id-ecPublicKey Public-Key: (384 bit) pub: 04:78:cb:47:0b:78:48:7a:ad:90:b1:d9:2d:4a:2f: d9:35:1f:cc:28:d6:af:4a:6d:c7:36:7e:ed:1a:88: 1f:a9:aa:a7:f0:04:a0:1c:86:bb:c9:45:3e:f8:fb: 28:0c:3e:a4:7f:ef:82:7b:bb:ac:77:49:90:3b:54: a7:75:82:16:8f:64:0b:88:8c:f4:35:91:fc:07:f4: 2b:e2:2e:c9:d0:82:b0:b1:09:54:9e:e9:d9:aa:fe: 4a:63:d4:cb:41:ad:27 ASN1 OID: secp384r1 NIST CURVE: P-384 Signature Algorithm: ecdsa-with-SHA256 Signature Value: 30:65:02:31:00:86:4e:33:e4:86:37:4c:26:a7:be:57:51:44: 8e:6c:88:ea:3c:03:58:00:a3:5e:7a:53:9e:2c:54:b3:ab:82: 25:fe:4c:e4:be:4d:8c:56:e2:da:d8:de:d2:20:ca:13:55:02: 30:0c:2a:27:a7:fd:2b:a9:87:4f:06:ea:4e:2d:cc:48:4d:9d: d7:cf:73:88:6d:98:54:18:83:6d:e5:a9:c3:84:75:c9:ee:c6: 0d:1a:15:a2:8c:68:86:88:83:17:b9:7a:9b The new certificate is good for 10 years. Certificate: Data: Version: 3 (0x2) Serial Number: 42:29:94:01:e1:cb:32:dc:f8:b4:64:6d:9e:1e:28:8d:7b:5a:53:3b Signature Algorithm: ecdsa-with-SHA256 Issuer: CN=test.example.com Validity Not Before: May 25 09:11:37 2024 GMT Not After : May 23 09:11:37 2034 GMT Subject: CN=test.example.com Subject Public Key Info: Public Key Algorithm: id-ecPublicKey Public-Key: (384 bit) pub: 04:78:cb:47:0b:78:48:7a:ad:90:b1:d9:2d:4a:2f: d9:35:1f:cc:28:d6:af:4a:6d:c7:36:7e:ed:1a:88: 1f:a9:aa:a7:f0:04:a0:1c:86:bb:c9:45:3e:f8:fb: 28:0c:3e:a4:7f:ef:82:7b:bb:ac:77:49:90:3b:54: a7:75:82:16:8f:64:0b:88:8c:f4:35:91:fc:07:f4: 2b:e2:2e:c9:d0:82:b0:b1:09:54:9e:e9:d9:aa:fe: 4a:63:d4:cb:41:ad:27 ASN1 OID: secp384r1 NIST CURVE: P-384 X509v3 extensions: X509v3 Subject Key Identifier: DA:49:EA:13:99:CA:DE:10:D2:70:2B:27:E2:60:AA:E0:F4:7B:EA:50 X509v3 Authority Key Identifier: DA:49:EA:13:99:CA:DE:10:D2:70:2B:27:E2:60:AA:E0:F4:7B:EA:50 X509v3 Basic Constraints: critical CA:TRUE Signature Algorithm: ecdsa-with-SHA256 Signature Value: 30:65:02:30:6d:4e:25:4f:84:f4:38:7e:c4:de:c8:d1:55:0c: af:4b:e4:c0:a1:f3:59:de:fb:48:0a:96:07:65:29:9f:fe:7c: 3c:ee:f0:c9:ca:17:bc:cd:bd:a4:31:38:24:4f:c6:e5:02:31: 00:e6:9a:ce:52:60:4b:b8:0e:e7:23:6d:8a:69:a0:21:e5:d1: bb:e8:e9:09:6a:32:d6:8c:58:49:f4:76:86:e6:c1:b8:24:d3: 44:08:fa:1c:ef:34:70:c1:24:76:a9:35:8f Signed-off-by: Lior Okman <lior.okman@sap.com> (cherry picked from commit c2c9b43) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: use Patch API for infra-client (#3034) * fix(infrastructure): use Patch API instead Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: add interceptor for ApplyPatch on fake client Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: trigger make generate Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: remove update verb Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: SetUID no longer needed Signed-off-by: Ardika Bagus <me@ardikabs.com> --------- Signed-off-by: Ardika Bagus <me@ardikabs.com> (cherry picked from commit cc01bf5) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * refactor: infra client CreateOrUpdate to ServerSideApply (#3134) * refactor(infra-client): CreateOrUpdate to ServerSideApply Signed-off-by: Ardika Bagus <me@ardikabs.com> * test(infra-client): add e2e test for ServerSideApply Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: remove comment Signed-off-by: Ardika Bagus <me@ardikabs.com> * chore: fix linter Signed-off-by: Ardika Bagus <me@ardikabs.com> --------- Signed-off-by: Ardika Bagus <me@ardikabs.com> (cherry picked from commit 81108f2) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: duplicated xroutes are added to gatewayapi.Resources (#3282) fix duplicated xroutes Signed-off-by: Dingkang Li <dingkang1743@gmail.com> (cherry picked from commit 32c6876) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: add proxy protocol always as first listenerFilter (#3332) * add proxy protocol always as first listenerFilter Signed-off-by: Jesse Haka <haka.jesse@gmail.com> * add test Signed-off-by: Jesse Haka <haka.jesse@gmail.com> --------- Signed-off-by: Jesse Haka <haka.jesse@gmail.com> (cherry picked from commit 6d8f2dc) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: security policy reference grant from field type (#3386) fix: security policy reference grant from field Signed-off-by: Eguzki Astiz Lezaun <eastizle@redhat.com> (cherry picked from commit bd72474) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * bug: Route extension filters with different types but the same name and namespace aren't correctly cached (#3388) * Route extension filters are unstructured.Unstructured instances, so caching them should be done with both the name and type as a key. Signed-off-by: Lior Okman <lior.okman@sap.com> * Moved NamespacedNameAndType to the Kubernetes helpers, and renamed it to be clearer about what it has. Signed-off-by: Lior Okman <lior.okman@sap.com> * Also renamed the helper function. Signed-off-by: Lior Okman <lior.okman@sap.com> * Moved to the 'utils' package to be beside NamespacedName. Signed-off-by: Lior Okman <lior.okman@sap.com> * Renamed structure according to review, and updated the comments Signed-off-by: Lior Okman <lior.okman@sap.com> --------- Signed-off-by: Lior Okman <lior.okman@sap.com> (cherry picked from commit 95e2e35) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix(translator): set ignoreCase for header matchers in extAuth (#3420) fix: set ignoreCase for header matchers in extAuth Signed-off-by: haoqixu <hq.xu0o0@gmail.com> (cherry picked from commit 8206e11) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix secrets/configmap updates do not trigger a controller reconcile (#3499) * ensure both secrets and config map reconcile upon changes ensure secret/config map changes trigger a reconcile Signed-off-by: Alex Volchok <alex.volchok@sap.com> * Update controller.go Signed-off-by: Alex Volchok <alex.volchok@sap.com> * Update controller.go Signed-off-by: Alex Volchok <alex.volchok@sap.com> --------- Signed-off-by: Alex Volchok <alex.volchok@sap.com> (cherry picked from commit ff2c598) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * feat: backend TLS SAN validation (#3507) * BTLS: enforce SAN validation Signed-off-by: Guy Daich <guy.daich@sap.com> * use dedicated cert for ext-proc e2e test Signed-off-by: Guy Daich <guy.daich@sap.com> * fix ext-proc server client tls settings Signed-off-by: Guy Daich <guy.daich@sap.com> --------- Signed-off-by: Guy Daich <guy.daich@sap.com> (cherry picked from commit dc201ba) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: ReplaceFullPath not working for root path (/) (#3530) * fix: ReplaceFullPath not working for root path (/) Takes #2817 forward Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 8f83c3d) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * chore: Remove namespace restriction for EnvoyProxy parametersRef reso… (#3544) chore: Remove namespace restriction for EnvoyProxy parametersRef resource (cherry picked from commit b870e39) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix test rm invalid diff related to #3134 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix GatewayInfraResourceTest Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Lior Okman <lior.okman@sap.com> Signed-off-by: Ardika Bagus <me@ardikabs.com> Signed-off-by: Dingkang Li <dingkang1743@gmail.com> Signed-off-by: Jesse Haka <haka.jesse@gmail.com> Signed-off-by: Eguzki Astiz Lezaun <eastizle@redhat.com> Signed-off-by: haoqixu <hq.xu0o0@gmail.com> Signed-off-by: Alex Volchok <alex.volchok@sap.com> Signed-off-by: Guy Daich <guy.daich@sap.com> Co-authored-by: Lior Okman <lior.okman@sap.com> Co-authored-by: Ardika <me@ardikabs.com> Co-authored-by: Dingkang Li <dingkang1743@gmail.com> Co-authored-by: Jesse Haka <haka.jesse@gmail.com> Co-authored-by: Eguzki Astiz Lezaun <eastizle@redhat.com> Co-authored-by: xu0o0 <hqcat6@gmail.com> Co-authored-by: Alex Volchok <alex.volchok@sap.com> Co-authored-by: Guy Daich <guy.daich@sap.com> Co-authored-by: zou rui <xiaorui.zou@gmail.com>

arkodg requested a review from a team as a code owner April 8, 2024 13:13

arkodg marked this pull request as draft April 8, 2024 13:13

zirain reviewed Apr 8, 2024

View reviewed changes

internal/cmd/egctl/testdata/translate/out/quickstart.all.yaml Outdated

Copy link
Copy Markdown

Member

zirain Apr 8, 2024

Choose a reason for hiding this comment

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

like this naming rule

Arko Dasgupta added 3 commits April 15, 2024 04:18

Use <proto>-<port> for naming service and container ports

318b7d9

Takes inspiration from envoyproxy#2973 to name port, not off the listener but off the port-proto ensuring that patch (during updates) also works Fixes: envoyproxy#3111 Signed-off-by: Arko Dasgupta <arko@tetrate.io>

testdata

8f2dd28

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

fix test

c79553b

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

arkodg force-pushed the port-name branch from c10a4c8 to c79553b Compare April 15, 2024 11:35

arkodg marked this pull request as ready for review April 15, 2024 11:36

arkodg requested review from a team and zirain April 15, 2024 11:36

zirain reviewed Apr 15, 2024

View reviewed changes

internal/gatewayapi/listener.go Outdated Show resolved Hide resolved

cnvergence approved these changes Apr 15, 2024

View reviewed changes

move to helper pkg

be14cd3

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

arkodg requested a review from zirain April 15, 2024 11:48

zirain previously approved these changes Apr 15, 2024

View reviewed changes

shawnh2 reviewed Apr 16, 2024

View reviewed changes

fix e2e

f0bd630

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

arkodg dismissed zirain’s stale review via f0bd630 April 16, 2024 06:19

arkodg requested a review from zirain April 16, 2024 06:22

Arko Dasgupta added 2 commits April 15, 2024 23:36

lint

13f88e8

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

fix e2e

7c1cb11

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

zirain approved these changes Apr 17, 2024

View reviewed changes

arkodg merged commit c41247b into envoyproxy:main Apr 17, 2024

arkodg deleted the port-name branch April 17, 2024 05:18

zetaab mentioned this pull request Apr 25, 2024

bug: envoy proxy pods restarted always when configuration change #2965

Closed

arkodg mentioned this pull request May 15, 2024

Envoy Gateways are restarted when a resource is updated #3394

Closed

arkodg added the cherrypick/release-v1.0.2 label May 29, 2024

arkodg mentioned this pull request Jun 7, 2024

DeltaAggregatedResources gRPC config stream to xds_cluster closed: 13, #3559

Closed

arkodg mentioned this pull request Jun 12, 2024

[release/v1.0] Cherry-pick fixes for v1.0.2 #3593

Merged

Conversation

arkodg commented Apr 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zirain Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

arkodg commented Apr 8, 2024

Uh oh!

cnvergence commented Apr 8, 2024

Uh oh!

owenhaynes commented Apr 9, 2024

Uh oh!

arkodg commented Apr 9, 2024

Uh oh!

owenhaynes commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arkodg commented Apr 9, 2024

Uh oh!

cnvergence commented Apr 9, 2024

Uh oh!

owenhaynes commented Apr 11, 2024

Uh oh!

Uh oh!

cnvergence left a comment

Choose a reason for hiding this comment

Uh oh!

zirain commented Apr 15, 2024

Uh oh!

zirain commented Apr 16, 2024

Uh oh!

shawnh2 left a comment

Choose a reason for hiding this comment

Uh oh!

arkodg commented Apr 16, 2024

Uh oh!

zirain commented Apr 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

arkodg commented Apr 8, 2024 •

edited

Loading

owenhaynes commented Apr 9, 2024 •

edited

Loading