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

Add regex function in string matcher #27984

Closed
wants to merge 4 commits into from
Closed

Add regex function in string matcher #27984

wants to merge 4 commits into from

Conversation

xulingqing
Copy link
Member

@xulingqing xulingqing commented Oct 15, 2020 •
edited

Add regex matching for Authz policies:

istio.io website change:

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[X] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Pull Request Attributes

Please check any characteristics that apply to this pull request.

[X] Does not have any changes that may affect Istio users.

@xulingqing xulingqing requested a review from a team as a code owner October 15, 2020 08:07
@istio-policy-bot istio-policy-bot added area/security release-notes-none Indicates a PR that does not require release notes. labels Oct 15, 2020
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Oct 15, 2020
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 15, 2020
@istio-testing istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 15, 2020
yangminzhu
yangminzhu reviewed Oct 15, 2020
View reviewed changes
Copy link
Contributor

@yangminzhu yangminzhu left a comment

Choose a reason for hiding this comment

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

You should update the tests under the builder, see all the golden files in testdata.

Also add a new global environment flag to make this behavior opt-in.

tests/integration/security/authorization_test.go Outdated Show resolved Hide resolved
pilot/pkg/security/authz/matcher/string.go Outdated Show resolved Hide resolved
@yangminzhu yangminzhu removed the release-notes-none Indicates a PR that does not require release notes. label Oct 15, 2020
@yangminzhu
Copy link
Contributor

yangminzhu commented Oct 15, 2020 •
edited

This is also user-facing changes (even guarded by flag and disabled by default), it needs an item for the release notes.

@xulingqing xulingqing requested review from a team as code owners October 16, 2020 08:44
@xulingqing xulingqing requested a review from a team as a code owner October 16, 2020 18:58
@istio-testing istio-testing added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2020
@istio-testing istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 16, 2020
@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2020
@yangminzhu
Copy link
Contributor

yangminzhu commented Oct 28, 2020 via email

@xulingqing
Copy link
Member Author

/retest

2 similar comments
@xulingqing
Copy link
Member Author

/retest

@xulingqing
Copy link
Member Author

/retest

@xulingqing xulingqing added the do-not-merge/hold Block automatic merging of a PR. label Oct 29, 2020
yangminzhu
yangminzhu reviewed Oct 29, 2020
View reviewed changes
tests/integration/security/testdata/authz/v1beta1-conditions.yaml.tmpl Outdated
---

# Each of the following authorization policy uses a different condition on the given path.

apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
name: condition-request-headers-notvalues
name: condition-request-headers
Copy link
Contributor

Choose a reason for hiding this comment

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

why change the name here? It's duplicate of the other policy and will override it.

@xulingqing
Copy link
Member Author

/retest

@howardjohn howardjohn mentioned this pull request Nov 3, 2020
Closed
@istio-testing
Copy link
Collaborator

@xulingqing: PR needs rebase.

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/test-infra repository.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Nov 8, 2020
@istio-testing
Copy link
Collaborator

@xulingqing: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
integ-pilot-multicluster-tests_istio 281b595 link /test integ-pilot-multicluster-tests_istio

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/test-infra repository. I understand the commands that are listed here.

@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Nov 29, 2020
@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2020-10-30. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Dec 14, 2020
@WesleySensedia
Copy link

@xulingqing xulingqing and @howardjohn. Any chance to bring this to life?

@Cakasim Cakasim mentioned this pull request Oct 11, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howardjohn howardjohn howardjohn left review comments

@yangminzhu yangminzhu yangminzhu left review comments

Assignees

@xulingqing xulingqing

Labels
area/security cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. do-not-merge/hold Block automatic merging of a PR. lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while needs-rebase Indicates a PR needs to be rebased before being merged size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@xulingqing @yangminzhu @istio-testing @istio-policy-bot @WesleySensedia @howardjohn