Skip to content

Conversation

sayboras
Copy link
Member

The previous PR #18478 wraps existing viper GetStringMapString function
to get around upstream bugs, however, it's unintentionally restricted a
few formats, which supported before in cilium, such as:

  • --aws-instance-limit-mapping=c6a.2xlarge=4,15,15,m4.xlarge=1,5,10
  • --api-rate-limit=endpoint-create=rate-limit:10/s,rate-burst:10,parallel-requests:10,auto-adjust:true

For complicated attribute, we are allowing comma character in value part
of key value pair. As golang didn't support look-ahead functionalities in
built-in regex library, this commit is to replace string.Split function
by custom implementation to handle such scenario.

Relates: #18478
Fixes: #18973
Signed-off-by: Tam Mach tam.mach@cilium.io

The previous PR cilium#18478 wraps existing viper GetStringMapString function
to get around upstream bugs, however, it's unintentionally restricted a
few formats, which supported before in cilium, such as:

- --aws-instance-limit-mapping=c6a.2xlarge=4,15,15,m4.xlarge=1,5,10
- --api-rate-limit=endpoint-create=rate-limit:10/s,rate-burst:10,parallel-requests:10,auto-adjust:true

For complicated attribute, we are allowing comma character in value part
of key value pair. As golang didn't support look-ahead functionalities in
built-in regex library, this commit is to replace string.Split function
by custom implementation to handle such scenario.

Relates: cilium#18478
Fixes: cilium#18973
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 25, 2022
@sayboras sayboras added the release-note/bug This PR fixes an issue in a previous release of Cilium. label May 25, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 25, 2022
@sayboras
Copy link
Member Author

/test

@sayboras
Copy link
Member Author

/test-runtime

@sayboras sayboras marked this pull request as ready for review May 25, 2022 07:05
@sayboras sayboras requested a review from a team as a code owner May 25, 2022 07:05
@jibi jibi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 26, 2022
@jibi jibi merged commit 070ded0 into cilium:master May 26, 2022
@sayboras sayboras deleted the tam/issue-18973 branch May 26, 2022 14:57
@joestringer joestringer added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cilium 1.11.2. API Rate limit stopped working
5 participants