Update headerExtractor yaml syntax (Breaking Change) #2767
Conversation
|
/ok-to-test |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenamed API-key extractor identifiers in docs/config from Changes
Sequence Diagram(s)(omitted — changes are documentation/annotation-oriented with a minor local setter behavior change; no new multi-component control flow to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/apikey/ApiKeysInterceptor.java`:
- Around line 38-46: Update the YAML example in ApiKeysInterceptor (the block
showing "api: port:2000 ...") to insert a space after the colon in the port
mapping so it reads "port: 2000"; ensure the change is made inside the YAML code
block in the ApiKeysInterceptor.java Javadoc/example so users can copy valid
YAML.
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/apikey/extractors/ApiKeyHeaderExtractor.java`:
- Line 38: The MCElement annotation on ApiKeyHeaderExtractor
(`@MCElement`(name="headerExtractor", component = false, collapsed = true))
requires collapsed YAML syntax; update the example YAML entries that use
expanded form (e.g., headerExtractor: {name: X-Key} or headerExtractor: {name:
"X-Api-Key"}) to the collapsed forms: use headerExtractor: X-Key for a custom
header or headerExtractor: {} for the default header; locate occurrences in the
API key example apis.yaml files (the RBAC and apikey-openapi examples) and
replace the expanded mappings accordingly so they parse with the collapsed=true
constraint.
...main/java/com/predic8/membrane/core/interceptor/apikey/extractors/ApiKeyHeaderExtractor.java
Outdated
Show resolved
Hide resolved
|
This pull request needs "/ok-to-test" from an authorized committer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@distribution/examples/security/api-key/simple/apis.yaml`:
- Line 17: The YAML contains a null for the collapsed element `headerExtractor`
which the collapsed-element parser forbids and causes
ParsingException("Collapsed element must not be null."); update the
`headerExtractor` entry (the one contrasted with `queryParamExtractor`) to be a
non-null scalar or an empty object — e.g., replace the null with a header name
like `X-Api-Key` or use the explicit empty object form `{}` so the parser
accepts the value for the `headerExtractor` element.
distribution/examples/security/jwt/apikey-to-jwt-conversion/apis.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/apikey/ApiKeysInterceptor.java`:
- Around line 38-46: The YAML example in ApiKeysInterceptor uses a scalar
extractor form ("- header: X-Api-Key") that is inconsistent with
ApiKeyHeaderExtractor's documented object forms; update the example in
ApiKeysInterceptor to use the object syntax shown in ApiKeyHeaderExtractor (e.g.
replace "- header: X-Api-Key" with "- header: { name: X-Api-Key }") so the
example matches the extractor's docs and the parser expectations, or
alternatively add collapsed = true to the `@MCElement` on ApiKeyHeaderExtractor if
you prefer to support the scalar form—refer to the ApiKeysInterceptor YAML
example and the ApiKeyHeaderExtractor `@MCElement` declaration to implement the
chosen change.
🧹 Nitpick comments (1)
core/src/main/java/com/predic8/membrane/core/interceptor/apikey/extractors/ApiKeyHeaderExtractor.java (1)
62-62: Silent fallback on blank input may mask misconfiguration.If a user accidentally sets
name: ""orname: " ", the setter silently retains the defaultX-Api-Keywith no warning. Consider logging a warning or throwing anIllegalArgumentExceptionso misconfigurations surface early rather than silently falling back to the default.Suggested improvement
`@MCAttribute`(attributeName = "name") public void setHeaderName(String headerName) { - if(!headerName.isBlank()) this.headerName = new HeaderName(headerName); + if (headerName.isBlank()) { + log.warn("Blank header name provided; using default '{}'", this.headerName.getName()); + return; + } + this.headerName = new HeaderName(headerName); }This would require adding a logger field (similar to
ApiKeyQueryParamExtractor):private static final Logger log = LoggerFactory.getLogger(ApiKeyHeaderExtractor.class);
core/src/main/java/com/predic8/membrane/core/interceptor/apikey/ApiKeysInterceptor.java
Show resolved
Hide resolved
…o improve-api-key-yaml-syntax
2 participants
Summary by CodeRabbit
header/queryacross docs and examples.