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.

Sign up for GitHub

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

Jump to bottom

Fix TestE2E/RateLimitBasedJwtClaims test #2097

Merged
merged 2 commits into from
Oct 28, 2023
Merged

Fix TestE2E/RateLimitBasedJwtClaims test #2097

merged 2 commits into from
Oct 28, 2023

Conversation

zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Oct 28, 2023 •
edited
Loading

Fix #2095
Related: #2086 #2096

@zhaohuabing zhaohuabing requested a review from a team as a code owner October 28, 2023 00:32
@zhaohuabing zhaohuabing changed the title Fix e2e Fix TestE2E/RateLimitBasedJwtClaims test Oct 28, 2023
zirain
zirain previously approved these changes Oct 28, 2023
View reviewed changes
Copy link
Contributor

@zirain zirain left a comment

Choose a reason for hiding this comment

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

LGTM, why it pass before?

@codecov
Copy link

codecov bot commented Oct 28, 2023 •
edited
Loading

Codecov Report

Merging #2097 (3acc7db) into main (4cb4547) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2097      +/-   ##
==========================================
+ Coverage   64.23%   64.26%   +0.03%     
==========================================
  Files         107      107              
  Lines       14626    14626              
==========================================
+ Hits         9395     9400       +5     
+ Misses       4666     4661       -5     
  Partials      565      565

see 2 files with indirect coverage changes

@zhaohuabing
Copy link
Member Author

zhaohuabing commented Oct 28, 2023 •
edited
Loading

LGTM, why it pass before?

Jwks remote fetch is failed

(base) ➜  ~ curl https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/authn/jwks.json                                 
404: Not Found%

I renamed the directory auth to jwt but forgot to change the path in the yaml files. The directory auth existed before that PR merged, so the e2e succeeded.

The interesting part is: If I had changed the yaml files in that PR, it wouldn't have passed the E2E test either, as the jwt directory didn't exist before that PR was merged.

@zhaohuabing
Revert "Skip RateLimitBasedJwtClaimsTest test (envoyproxy#2096)"
36e7161 
This reverts commit ef7a2a4.

fix e2e test

Signed-off-by: huabing zhao <[email protected]>
@zhaohuabing zhaohuabing requested a review from arkodg October 28, 2023 00:46
@zirain
Copy link
Contributor

zirain commented Oct 28, 2023

/retest

@zhaohuabing
Copy link
Member Author 

this policy can only have a targetRef.kind of Gateway

@arkodg Route target has been disabled in this release?

@zhaohuabing zhaohuabing requested a review from zirain October 28, 2023 01:26
@arkodg
Copy link
Contributor

arkodg commented Oct 28, 2023

gateway/api/v1alpha1/securitypolicy_types.go

Line 40 in 4cb4547

// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway'", message="this policy can only have a targetRef.kind of Gateway"
seems like a bug :(

@zhaohuabing
fix XValidation
3acc7db 
Signed-off-by: huabing zhao <[email protected]>
@arkodg
Copy link
Contributor

arkodg commented Oct 28, 2023

it should look like

gateway/api/v1alpha1/backendtrafficpolicy_types.go

Line 40 in 4cb4547

// +kubebuilder:validation:XValidation:rule="self.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute']", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute"

@zhaohuabing
Copy link
Member Author

zhaohuabing commented Oct 28, 2023 •
edited
Loading

it should look like

gateway/api/v1alpha1/backendtrafficpolicy_types.go

Line 40 in 4cb4547

// +kubebuilder:validation:XValidation:rule="self.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute']", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute"

I just added Gateway, HTTPRoute, and GRPCRoute in the SecurityPolicy. I think SecurityPolicy is just for layer 7, right?

@zhaohuabing
Copy link
Member Author

/retest

@arkodg
Copy link
Contributor

arkodg commented Oct 28, 2023 •
edited
Loading

it should look like

gateway/api/v1alpha1/backendtrafficpolicy_types.go

Line 40 in 4cb4547

// +kubebuilder:validation:XValidation:rule="self.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute']", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute"

I just added Gateway, HTTPRoute, and GRPCRoute in the SecurityPolicy. I think SecurityPolicy is just for layer 7, right?

with cors, and jwt those features are NA for TCPRoute, TLSRoute & UDPRoute, so we can skip for now

@zhaohuabing
Copy link
Member Author

The Kubernetes integration test is flake, weird.

@zhaohuabing
Copy link
Member Author

/retest

arkodg
arkodg approved these changes Oct 28, 2023
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg merged commit f301527 into envoyproxy:main Oct 28, 2023
20 checks passed
arkodg pushed a commit to arkodg/gateway that referenced this pull request Nov 1, 2023
@zhaohuabing @arkodg
Fix TestE2E/RateLimitBasedJwtClaims test (envoyproxy#2097)
62fbd2a 
* Revert "Skip RateLimitBasedJwtClaimsTest test (envoyproxy#2096)"

This reverts commit ef7a2a4.

fix e2e test

Signed-off-by: huabing zhao <[email protected]>

* fix XValidation

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit f301527)
Signed-off-by: Arko Dasgupta <[email protected]>
arkodg added a commit that referenced this pull request Nov 1, 2023
@arkodg @zhaohuabing
@Xunzhuo @dependabot @zirain @slayer321
[release/v0.6] Cherry-pick fixes into v0.6 (#2150)
c62a470 
* Fix TestE2E/RateLimitBasedJwtClaims test (#2097)

* Revert "Skip RateLimitBasedJwtClaimsTest test (#2096)"

This reverts commit ef7a2a4.

fix e2e test

Signed-off-by: huabing zhao <[email protected]>

* fix XValidation

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit f301527)
Signed-off-by: Arko Dasgupta <[email protected]>

* add a newer PR reference in rc release docs (#2101)

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit fae8cd8)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: panic when using nil xdsRouteAction (#2104)

Signed-off-by: bitliu <[email protected]>
(cherry picked from commit 77445de)
Signed-off-by: Arko Dasgupta <[email protected]>

* remove cors, jwt, rl assignment from route translator (#2105)

All the translations and assignments now happen in the policy
translators

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit d65ab59)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: use lowercases of eg admin config fields (#2107)

(cherry picked from commit ef5e450)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: testGatewayClassWithParamRef e2e test flaky (#2114)

(cherry picked from commit 4047268)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump sigs.k8s.io/yaml from 1.3.0 to 1.4.0 (#2118)

Bumps [sigs.k8s.io/yaml](https://github.com/kubernetes-sigs/yaml) from 1.3.0 to 1.4.0.
- [Release notes](https://github.com/kubernetes-sigs/yaml/releases)
- [Changelog](https://github.com/kubernetes-sigs/yaml/blob/master/RELEASE.md)
- [Commits](kubernetes-sigs/yaml@v1.3.0...v1.4.0)

---
updated-dependencies:
- dependency-name: sigs.k8s.io/yaml
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit bf93a4e)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump github.com/bufbuild/buf from 1.27.1 to 1.27.2 in /tools/src/buf (#2121)

build(deps): bump github.com/bufbuild/buf in /tools/src/buf

Bumps [github.com/bufbuild/buf](https://github.com/bufbuild/buf) from 1.27.1 to 1.27.2.
- [Release notes](https://github.com/bufbuild/buf/releases)
- [Changelog](https://github.com/bufbuild/buf/blob/main/CHANGELOG.md)
- [Commits](bufbuild/buf@v1.27.1...v1.27.2)

---
updated-dependencies:
- dependency-name: github.com/bufbuild/buf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 58146ff)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump github.com/go-logr/logr from 1.2.4 to 1.3.0 (#2117)

Bumps [github.com/go-logr/logr](https://github.com/go-logr/logr) from 1.2.4 to 1.3.0.
- [Release notes](https://github.com/go-logr/logr/releases)
- [Changelog](https://github.com/go-logr/logr/blob/master/CHANGELOG.md)
- [Commits](go-logr/logr@v1.2.4...v1.3.0)

---
updated-dependencies:
- dependency-name: github.com/go-logr/logr
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit d1cbf5e)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump envoyproxy/toolshed from actions-v0.0.25 to 0.1.2 (#2116)

Bumps [envoyproxy/toolshed](https://github.com/envoyproxy/toolshed) from actions-v0.0.25 to 0.1.2. This release includes the previously tagged commit.
- [Release notes](https://github.com/envoyproxy/toolshed/releases)
- [Commits](envoyproxy/toolshed@actions-v0.0.25...actions-v0.1.2)

---
updated-dependencies:
- dependency-name: envoyproxy/toolshed
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 7b6344d)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump actions/setup-node from 3 to 4 (#2115)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 3 to 4.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v3...v4)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 5523099)
Signed-off-by: Arko Dasgupta <[email protected]>

* chore: add more EnvoyProxy cases (#2120)

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 268a6ef)
Signed-off-by: Arko Dasgupta <[email protected]>

* build(deps): bump github.com/golangci/golangci-lint from 1.55.0 to 1.55.1 in /tools/src/golangci-lint (#2119)

build(deps): bump github.com/golangci/golangci-lint

Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.55.0 to 1.55.1.
- [Release notes](https://github.com/golangci/golangci-lint/releases)
- [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
- [Commits](golangci/golangci-lint@v1.55.0...v1.55.1)

---
updated-dependencies:
- dependency-name: github.com/golangci/golangci-lint
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit be86295)
Signed-off-by: Arko Dasgupta <[email protected]>

* feat: CEL Validation in BackendTrafficPolicy (#2110)

* feat: CEL Validation in BackendTrafficPolicy

Signed-off-by: slayer321 <[email protected]>

* leastRequest with consistentHash nil test

Signed-off-by: slayer321 <[email protected]>

---------

Signed-off-by: slayer321 <[email protected]>
(cherry picked from commit b45ae24)
Signed-off-by: Arko Dasgupta <[email protected]>

* e2e: eg controlplane metrics (#2106)

* e2e: eg controlplane metrics

Signed-off-by: zirain <[email protected]>

* update

Signed-off-by: zirain <[email protected]>

* update

Signed-off-by: zirain <[email protected]>

* update name

Signed-off-by: zirain <[email protected]>

* rename

Signed-off-by: zirain <[email protected]>

* wait more

Signed-off-by: zirain <[email protected]>

* add comment

Signed-off-by: zirain <[email protected]>

* update

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 046a593)
Signed-off-by: Arko Dasgupta <[email protected]>

* chore: fix http2_protocol_options  warning message (#2048)

chore:fix http2_protocol_options warning message

Signed-off-by: zhaonan <[email protected]>
(cherry picked from commit 179d265)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix Failed to update SecurityPolicy status (#2128)

fix #2127

Signed-off-by: Huabing Zhao <[email protected]>
(cherry picked from commit 57e1aec)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: add missing status equal for SecurityPolicy (#2134)

(cherry picked from commit 71c09f2)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix jwt doc (#2135)

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit df1e209)
Signed-off-by: Arko Dasgupta <[email protected]>

* Bump Gateway API to v1.0.0 (#2142)

(cherry picked from commit 3dd0ee7)
Signed-off-by: Arko Dasgupta <[email protected]>

* Enable HTTPRouteRewritePath test (#2112)

* Enable HTTPRouteRewritePath test

Fixes: #2004

Signed-off-by: Arko Dasgupta <[email protected]>

* fix prefix match

Signed-off-by: Arko Dasgupta <[email protected]>

* make testdata

Signed-off-by: Arko Dasgupta <[email protected]>

* fix path match

Signed-off-by: Arko Dasgupta <[email protected]>

* rm trailing /

Signed-off-by: Arko Dasgupta <[email protected]>

* sort on path match type

Signed-off-by: Arko Dasgupta <[email protected]>

* make testdata

Signed-off-by: Arko Dasgupta <[email protected]>

* temp var

Signed-off-by: Arko Dasgupta <[email protected]>

* fix url rewrite

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 2b3bc9f)
Signed-off-by: Arko Dasgupta <[email protected]>

* helm: remove kube-rbac-proxy and update metrics service (#2108)

(cherry picked from commit 0c5b054)
Signed-off-by: Arko Dasgupta <[email protected]>

* doc: user doc for CORS (#2137)

* cors docs

Signed-off-by: huabing zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit d0dc987)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix(ci): bump go version to 1.21 (#2144)

* fix(ci): bump go version to 1.21

Signed-off-by: bitliu <[email protected]>

* update

Signed-off-by: bitliu <[email protected]>

---------

Signed-off-by: bitliu <[email protected]>
(cherry picked from commit 6e81fb3)
Signed-off-by: Arko Dasgupta <[email protected]>

* conformance: Enable HTTPRouteBackendProtocolH2C conformance test (#2136)

* nit

Signed-off-by: zirain <[email protected]>

* build Cluster depends on route type and service appProtocol

Signed-off-by: zirain <[email protected]>

* enable HTTPRouteBackendProtocolH2C conformance test

Signed-off-by: zirain <[email protected]>

* lint

Signed-off-by: zirain <[email protected]>

* fix

Signed-off-by: zirain <[email protected]>

* address comment

Signed-off-by: zirain <[email protected]>

* move to package internal/ir

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 6d532c1)
Signed-off-by: Arko Dasgupta <[email protected]>

* Fix attachedRoutes computation (#2085)

* Fix attachedRoutes computation

* Fixes: #2077
* Fixes: #1916

Signed-off-by: Arko Dasgupta <[email protected]>

* make testdata

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 93a12e7)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix comments in loadbalancer api (#2145)

Signed-off-by: zhaonan <[email protected]>
(cherry picked from commit 77bcb3c)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix(docs): incorrect quickstart links (#2146)

Signed-off-by: bitliu <[email protected]>
(cherry picked from commit defed57)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: null pointer when CORS maxAge is not specified (#2133)

fix nullpointer

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
(cherry picked from commit 35c4fea)
Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: bitliu <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: slayer321 <[email protected]>
Signed-off-by: zhaonan <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Co-authored-by: Huabing Zhao <[email protected]>
Co-authored-by: Xunzhuo <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Sachin Maurya <[email protected]>
Co-authored-by: tmsnan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arkodg arkodg arkodg approved these changes

@zirain zirain Awaiting requested review from zirain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix flaky TestE2E/RateLimitBasedJwtClaims test
3 participants
@zhaohuabing @zirain @arkodg