1.12.0 preparation#130
Conversation
feat: support use allow* multiple times in env, flag and docker labels
Signed-off-by: Wolfgang Ellsässer <git2026@wollomatic.dev>
Bumps golang from `c2a1f7b` to `f853308`. --- updated-dependencies: - dependency-name: golang dependency-version: 1.26.2-alpine3.23 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.17.0 to 2.19.0. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](step-security/harden-runner@f808768...8d3c67d) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-version: 2.19.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bump golang from `c2a1f7b` to `f853308`
…p-security/harden-runner-2.19.0 Bump step-security/harden-runner from 2.17.0 to 2.19.0
…-times Feature/99 use allow multiple times
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/config/config_test.go`:
- Around line 102-117: The helper regexMapsEqual is flaky because it compares
slices positionally (aRegexes[i] vs bRegexes[i]) while the source
(extractLabelData iterating cntr.Labels) can produce regexes in different
orders; update regexMapsEqual to normalize both slices before comparison by
building slices of the regex string representations for each method, sorting
them (e.g., sort.Strings) and then comparing lengths and each element, ensuring
the comparison uses ar.String() values rather than relying on original slice
order.
In `@internal/config/config.go`:
- Around line 693-699: The allowSpec parsing currently uses slices.ContainsFunc
with strings.HasPrefix which lets tokens like "GETTING" match "GET" and then
stores a regex under the wrong method; change parsing to split allowSpec on '.'
first (use strings.Cut or strings.SplitN) to extract the first token, validate
that exact token equals one of supportedHTTPMethods (not prefix-match), and only
then call compileRegexp(labelValue, method, "docker container label") and
proceed; update the code paths that reference allowSpec, supportedHTTPMethods,
method, labelValue, and compileRegexp so the extracted method is strictly
validated before compiling/storing the regex.
In `@README.md`:
- Around line 96-101: Reword the multi-value examples to be clearer and more
grammatical: replace awkward phrases like "use allow GET multiple times" with
explicit wording such as "supports using -allowGET multiple times" and change
"can set multiple -allow*" to "supports multiple -allow* entries"; update the
specific example lines mentioning '-allowGET', 'SP_ALLOW_GET', 'SP_ALLOW_GET_2',
and 'SP_ALLOW_HEAD' (and the similar lines at 109-110) to use this clearer
phrasing so readers understand that multiple -allowGET flags or multiple
SP_ALLOW_GET environment variables are supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbd04e42-3c78-4abb-bf81-c027cf46cb7f
📒 Files selected for processing (12)
.github/workflows/dependency-review.ymlDockerfileREADME.mdcmd/socket-proxy/handlehttprequest.gointernal/config/config.gointernal/config/config_test.gointernal/config/env.gointernal/config/env_test.gointernal/config/param.gointernal/docker/api/types/events/events.gointernal/docker/api/types/filters/parse.gointernal/go-connections/sockets/sockets.go
💤 Files with no reviewable changes (3)
- internal/go-connections/sockets/sockets.go
- internal/docker/api/types/events/events.go
- internal/docker/api/types/filters/parse.go
| func regexMapsEqual(a, b map[string][]*regexp.Regexp) bool { | ||
| if len(a) != len(b) { | ||
| return false | ||
| } | ||
| for method, aRegexes := range a { | ||
| bRegexes, ok := b[method] | ||
| if !ok || len(aRegexes) != len(bRegexes) { | ||
| return false | ||
| } | ||
| for i, ar := range aRegexes { | ||
| if ar.String() != bRegexes[i].String() { | ||
| return false | ||
| } | ||
| } | ||
| } | ||
| return true |
There was a problem hiding this comment.
This helper makes Test_extractLabelData flaky.
extractLabelData() iterates cntr.Labels, so the order of GET regexes is not stable. Comparing aRegexes[i] to bRegexes[i] will intermittently fail when socket-proxy.allow.get.1 is visited before .0. Normalise the slices before comparing.
Proposed fix
import (
"flag"
"math"
"os"
"reflect"
"regexp"
+ "slices"
"strconv"
"testing"
@@
func regexMapsEqual(a, b map[string][]*regexp.Regexp) bool {
if len(a) != len(b) {
return false
}
for method, aRegexes := range a {
bRegexes, ok := b[method]
if !ok || len(aRegexes) != len(bRegexes) {
return false
}
- for i, ar := range aRegexes {
- if ar.String() != bRegexes[i].String() {
- return false
- }
- }
+ aStrings := make([]string, len(aRegexes))
+ bStrings := make([]string, len(bRegexes))
+ for i, re := range aRegexes {
+ aStrings[i] = re.String()
+ }
+ for i, re := range bRegexes {
+ bStrings[i] = re.String()
+ }
+ slices.Sort(aStrings)
+ slices.Sort(bStrings)
+ if !reflect.DeepEqual(aStrings, bStrings) {
+ return false
+ }
}
return true
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/config/config_test.go` around lines 102 - 117, The helper
regexMapsEqual is flaky because it compares slices positionally (aRegexes[i] vs
bRegexes[i]) while the source (extractLabelData iterating cntr.Labels) can
produce regexes in different orders; update regexMapsEqual to normalize both
slices before comparison by building slices of the regex string representations
for each method, sorting them (e.g., sort.Strings) and then comparing lengths
and each element, ensuring the comparison uses ar.String() values rather than
relying on original slice order.
| if slices.ContainsFunc(supportedHTTPMethods, func(method string) bool { | ||
| // allowSpec starts with the method name like socket-proxy.allow.get.1 | ||
| return strings.HasPrefix(allowSpec, method) | ||
| }) { | ||
| // extract the method name from allowSpec | ||
| method, _, _ := strings.Cut(allowSpec, ".") | ||
| r, err := compileRegexp(labelValue, method, "docker container label") |
There was a problem hiding this comment.
Make label-method matching exact, not prefix-based.
This accepts typos like socket-proxy.allow.getting because GETTING starts with GET, then stores the regex under method GETTING, which is silently ignored at request time. Split on . first and validate the extracted method token exactly against supportedHTTPMethods.
Proposed fix
- if slices.ContainsFunc(supportedHTTPMethods, func(method string) bool {
- // allowSpec starts with the method name like socket-proxy.allow.get.1
- return strings.HasPrefix(allowSpec, method)
- }) {
- // extract the method name from allowSpec
- method, _, _ := strings.Cut(allowSpec, ".")
+ method, _, _ := strings.Cut(allowSpec, ".")
+ if slices.Contains(supportedHTTPMethods, method) {
r, err := compileRegexp(labelValue, method, "docker container label")
if err != nil {
return nil, nil, err
}
allowedRequests[method] = append(allowedRequests[method], r)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/config/config.go` around lines 693 - 699, The allowSpec parsing
currently uses slices.ContainsFunc with strings.HasPrefix which lets tokens like
"GETTING" match "GET" and then stores a regex under the wrong method; change
parsing to split allowSpec on '.' first (use strings.Cut or strings.SplitN) to
extract the first token, validate that exact token equals one of
supportedHTTPMethods (not prefix-match), and only then call
compileRegexp(labelValue, method, "docker container label") and proceed; update
the code paths that reference allowSpec, supportedHTTPMethods, method,
labelValue, and compileRegexp so the extracted method is strictly validated
before compiling/storing the regex.
Summary by CodeRabbit
Release Notes
New Features
-shutdowngracetimeparameter for shutdown configuration.Documentation
Chores