[WIP] feat: unified config#1514
Open
siarhei-fedziukovich wants to merge 171 commits into
Open
Conversation
Add the 10-document proposal for unifying DIAL Core configuration management — Configuration API + dial-cli — under docs/sandbox/dial-unified-config/. Covers problem context, architecture, API reference, security/audit, CLI design, user guide, migration & rollout, open questions, and admin MCP spec. Status v2.20: decisions locked, ready for Phase 1 implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Execution playbook for the Phase 1-3 (+4 NTH) MVP: slice register, agent loop, simplification principles, branching model, halt conditions, LSP integration notes. Companion to the proposal docs (01-09); spec stays the contract, IMPLEMENTATION.md governs execution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Project-scoped slash command at .claude/commands/dial-mvp.md that resumes the dial-unified-config MVP implementation in any new session. Loads context from IMPLEMENTATION.md + project memory, runs the agent loop, halts at architect-plan and on any halt-condition trigger. Usage: /dial-mvp [<slice-id> | status]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add .classpath, .factorypath, .project, .settings/ to .gitignore. These are generated by Eclipse and the JDTLS language server (used by Claude Code's LSP integration), should not be tracked. Existing entries cover IntelliJ (.idea/) and VS Code (.vscode/). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The .claude/commands/review-unified-config.md file is a personal/local slash command, not part of the shared MVP tooling. Gitignored to avoid accidental inclusion in commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refine the branch-hygiene policy in IMPLEMENTATION.md §3.2 / §9. Default remains rebase (linear history; force-push; sub-branches rebase onto new tip). Per-sync merge override is now allowed when situational — early in MVP with few slices in flight, or when conflicts resolve more cleanly with a merge commit. Late in MVP, prefer rebase to keep history readable for the final big-PR review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-DELETE locks - config.reload.onInvalidEntity default flipped from skip to abort (aligns reload-side with config.write.softValidation: false; opt-in skip retained for scale-up resilience). - Singleton /v1/settings/platform/global gains DELETE to release API control and revert the projection to file-sourced (or default); POST still 405. Three-state source field: api | file | default. Touches docs 01, 02, 03, 05, 06, 07, 08, 09, plus IMPLEMENTATION.md slice rows 1S.3, 2S.9, 3S.2 and the locked-rules summary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Git refuses to create refs/heads/feature/unified-config/<x> while refs/heads/feature/unified-config exists as a branch (ref-vs-directory conflict). Rename slice sub-branches from feature/unified-config/<id>-<short-title> to feature/unified-config-<id>-<short-title>. Wildcard ergonomics preserved (git branch --list 'feature/unified-config-*'). Discovered on first /dial-mvp 1S.0 run when the orchestrator hit halt-condition §4.1 #1 (constraint contradicts plan) and surfaced three options; Option A picked (smallest blast radius — no rename of an existing branch). Updates IMPLEMENTATION.md §3.2 + §9 and the /dial-mvp slash command. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce, EntityBucketBinding (#1513) Co-authored-by: SiarheiFedziukovich <siarhei_fedziukovich@epam.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slices now integrate via local `git merge --squash` into feature/unified-config — no per-slice PR, no per-slice formal code-owner review. Orchestrator halts for the user to approve the slice diff and a §3.5-formatted commit message before merging, then deletes the sub-branch and updates the slice register Status to merged. The single big PR feature/unified-config → development at MVP-complete remains the only formal external review checkpoint. Adds IMPLEMENTATION.md §3.5 (commit-message format with type guide and example), updates §3.2 (branching diagram + integration bullet), §4 (drops per-slice /ultrareview recommendation; reserved for MVP-complete or ad-hoc), §5 (status legend: in-review → awaiting-merge; PR column → Commit), and §9 (decisions log). Updates /dial-mvp slash command step 7 (OPEN PR → MERGE LOCALLY) and Important notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ction
Replaces the 1S.0 stub's 405 fall-through with a model-read handler off the
in-memory volatile Config. Public/Owner field projection: status is always
"valid" in Phase 1; source ("file") is admin-only. Empty name returns 404 —
the listing slot is reserved for 1S.2. Unblocks Track B (CLI 1C.0) on wire
contract.
Design anchors: 03 §1, §2, §4; 04 §1.5
Tests: server/src/test/java/com/epam/aidial/core/server/ConfigModelReadTest.java
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New project-scoped slash command that runs multiple dial-mvp slices sequentially. The two routine halts (architect-plan approval, merge-diff approval) become conditional, gated by self-tests in IMPLEMENTATION.md §4.2 — auto-approval is earned, not assumed. Halt conditions §4.1 still always trigger; auto-mode never bypasses them. Adds IMPLEMENTATION.md §4.2 (Auto-mode policy) with the §A ARCHITECT and §B MERGE LOCALLY self-test checklists, when-to-use guidance (mechanical/semi-mechanical only — Phase-3 sweep, Phase-2 prereqs), when-NOT-to-use guidance (1S.0, 2S.8, 2S.10, 4S.0 — high-uncertainty slices need plain /dial-mvp), and auditability format. Usage: /dial-mvp-auto (until blocked) | /dial-mvp-auto 3 (count) | /dial-mvp-auto until-phase-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the per-bucket listing endpoint behind the existing CONFIG_RESOURCE route. Phase 1 returns the full in-memory snapshot with hasMore: false (per 03 §4 forward-compat); ?limit shape-validated, ?cursor accepted-and- ignored. Public/Owner field projection on items is shared with the single- GET path via a projectModelItem helper. Trailing slash optional. Also fixes RegexUtil.collectGroups to skip optional named groups whose start is -1, which surfaced as a server-side IOOBE only when 1S.2 exercised /v1/models/public without a trailing slash. Design anchors: 03 §1, §4 Tests: server/src/test/java/.../ConfigModelListTest.java (new); ConfigModelReadTest update Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends ConfigResourceController GET dispatch to interceptors, roles, keys,
routes (platform/), schemas (public/), and the singleton settings at
platform/global. Bucket-aware authz already gates non-admin off platform/.
Key.key is masked with "***" — Phase 1 has no reveal-secrets surface. Settings
GET projects globalInterceptors with file/default source; POST/PUT/DELETE
return 405 with Allow: GET, PUT, DELETE.
Design anchors: 03 §1; 04 §1.2
Tests: server/src/test/java/com/epam/aidial/core/server/Config{Interceptor,Role,Key,Route,Schema,Settings}Test.java
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ResourceController overrides handle(resourceUrl) to add an additive admin admit on GETs to applications/toolsets. When the unified-config gate grants admin access, the request bypasses the rules-based AccessService check and proceeds with hasWriteAccess=true (full data); everyone else falls through to the existing flow unchanged. Phase 1 reads only — write preflight ships in 1S.5. Design anchors: 03 §1; 02 §6 Tests: server/src/test/java/com/epam/aidial/core/server/ConfigAdminAppToolsetReadTest.java Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AccessControlBaseController.handle(resourceUrl) gains a ConfigAuthorizationService preflight: when the admin role is asserted AND the unified-config gate authorizes the request (public/ + admin always; user-bucket only when admin owns it), the rules-based AccessService check is skipped and the handler runs with hasWriteAccess=true. Otherwise the request falls through to the existing path. Covers all 4 subclasses (Resource + 3 FILES) and both reads/writes; supersedes the narrower 1S.4 override on ResourceController. OQ-33's "admin can't reach user buckets" is enforced by the gate not admitting admin onto user buckets; existing share-based grants (publication review) continue through the rules path unchanged. Design anchors: 03 §1; OQ-21; OQ-33 Tests: server/src/test/java/com/epam/aidial/core/server/ConfigAdminPreflightTest.java Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds AdminExportController + RouteTemplate.CONFIG_EXPORT for /v1/admin/export. Admin-only via ConfigAuthorizationService; default JSON, YAML when ?format=yaml or Accept: application/yaml. Keys are re-attached with masked secrets — Config field's @JsonProperty(WRITE_ONLY) suppresses the map at serialization time. JSON-string round-trip avoids the TokenBuffer/writeRaw incompatibility in applicationTypeSchemas' custom serializer. Design anchors: 03 §1; 07 Phase 1 Tests: server/src/test/java/com/epam/aidial/core/server/AdminExportTest.java Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds AdminHealthConfigController + RouteTemplate.CONFIG_HEALTH for
/v1/admin/health/config. Admin-only via ConfigAuthorizationService; Phase 1
always reports {status:"healthy",skipped:[]} unconditionally — the invalid-
entity sibling store that populates skipped, plus the dial_config_skipped_*
Prometheus metrics, ship together in slice 2S.9. Cardinality-zero metric
scaffolds skipped here per §2.1/§2.3 (no observable behavior in Phase 1).
Design anchors: 07 Phase 2; 02 §4.1
Tests: server/src/test/java/com/epam/aidial/core/server/AdminHealthConfigTest.java
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r handler
testAdminCanReachPlatformEntity asserted the 1S.0 stub's 405 ("no handler yet")
on admin GET /v1/interceptors/platform/anything. After slice 1S.3 wired the
interceptors read handler, the same request now resolves to 404 (interceptor
"anything" not found) — still proves the admin gate admitted. Adjusted the
expectation; comment notes the success signal can be 404 or 405 depending on
whether the type has its handler yet.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
§A self-test item 4 read literally would halt every Phase-2 prereq slice (pure-internal refactors with no HTTP surface) — contradicting §4.2's own "When to invoke" list that names them as auto-eligible. Loosen the wording to require integration tests only when the slice exposes HTTP behaviour; well-targeted unit tests otherwise. No behavioural change to the merge gate; the amendment unblocks the 2S.0-pre … 2S.7-pre auto-mode batch. Design anchors: IMPLEMENTATION.md §4.2 §A; §8 doc-amendment lifecycle Tests: no new tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces unconditional value.setKey(apiKey) with a blank-guarded call so API-managed keys (whose Key.key already holds the decrypted secret) are not silently overwritten by the human-readable map key. Legacy file-sourced format (map key = secret, Key.key blank) is unaffected. Compile-time prereq for the Phase-2 keys controller. Design anchors: 07 Phase 2 prereqs (line 98); OQ-12 Tests: server/src/test/java/com/epam/aidial/core/server/security/ApiKeyStoreTest.java Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
siarhei-fedziukovich
requested review from
alexey-ban,
nepalevov and
tataranovich
as code owners
… upstream fixes" This reverts commit 9bd05d6.
Contributor
Author
|
/deploy-review
|
Contributor
Dependency ReviewThe following issues were found:
Vulnerabilitiessettings.gradle
License Issuessettings.gradle
OpenSSF ScorecardScorecard details
Scanned Files
|
ConfigResourceController.handleGet lambdas project the canonical Config map key as `name` for API entries; respondList / handleSchemaGet listing branches dedup rows by full map key so file/API simple-name twins appear as distinct rows. Schema single-entity GET canonical match also emits the canonical name. Fallback in single-GET preserved (file entries remain GET-able). Tests flipped where they locked the old simple-name shape. Design anchors: 03 §4 (amended); locked-decision entry in project_unified_config_review.md Tests: server/src/test/.../MergedConfigStoreApiTest.java (+ regression guard testFileAndApiTwinsAppearAsSeparateListingRows), CanonicalIdListingTest, ModelWriteApiTest Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ope retrospective Backfills commit SHA and the architect-plan halt-at-scope note: original "drop fallback" plan would have broken ~10 fixture-using test classes by making file entries listing-only; user picked option (b') — keep GET fallback and fix listing dedup + projection only — to localize the change and avoid an operator UX regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on-on-PUT
Three fixes from /dial-uc-debug round 2026-05-08 issues 2/4/5:
(2) DialCli global flags now carry ScopeType.INHERIT so `-o yaml`,
`--env`, etc. bind after the subcommand chain — pre-Cli.3 they
were silently ignored when placed late on the command line.
(4) EntityReader.formatHttpError translates 404/409/412 into
`Not found: <id>` / `Already exists: <id>` / `Stale ETag: <id>`
stderr lines; reused from EntityWriter so write paths get the
same treatment. Bare `HTTP 404` (empty body) is gone.
(5) EntityWriter.updateEntity strips controller-projected fields
(`name`, `status`, `source`, `validationWarnings`) from the GET
response before merging `--set` and PUT-ing. Pre-Cli.3 the PUT
body included `status: "valid"` which the server's BLOB_MAPPER
rejected as Unrecognized.
Default table shape adds SOURCE + STATUS columns so file vs api entries
are visible at a glance (Issue 1 surfaces in CLI here; server side
shipped in Polish.1).
ModelCommandTest 7 friendly-format assertions + 1 stripped-name guard
+ 1 new -o-after-subcommand test.
Design anchors: 06 §2.4, 04 §1.5, 03 §4
Tests: cli/src/test/.../ModelCommandTest.java
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| * conclusive evidence of API origin. | ||
| */ | ||
| private boolean fromApi(String key) { | ||
| return key.startsWith(entityType + "/" + bucket + "/"); |
There was a problem hiding this comment.
If config file has e.g. key like below
"keys/platform/my-key": {
"project": "test",
"secured": false,
"roles": [
"admin"
]
}
its source will be api instead of file
| /** Extract the simple name from a canonical ID ("models/public/gpt-4" → "gpt-4"); pass through otherwise. */ | ||
| private static String simpleName(String key) { | ||
| int slash = key.lastIndexOf('/'); | ||
| return slash < 0 ? key : key.substring(slash + 1); |
There was a problem hiding this comment.
If config file has e.g. key like below
"keys/platform/my-key": {
"project": "test",
"secured": false,
"roles": [
"admin"
]
}
its name will be shortened
Contributor
Author
|
/deploy-review
|
Sourced from /dial-uc-debug round 2026-05-09 (Issue 1):
`dial-cli model validate --name X --from-file sample/dial-cli/manifests/06-model.yaml`
failed with `Unrecognized field "kind" (class Model)` because the CLI was
treating the {kind,name,spec} envelope as the raw model body. Same bug shape
on `model add --from-file <manifest>`. The discoverable workflow ("grab the
sample manifest, validate it") collided with --from-file's undocumented
"raw-spec only" contract.
Fix: EntityWriter.loadSpecOrFail detects manifest-envelope shape
({kind, name?, spec} — same shape used by `dial-cli apply -f` and shipped
in sample/dial-cli/manifests/*.yaml), validates `kind` matches the command's
expected kind, warns when the envelope's `name` differs from --name (flag
stays authoritative — same envelope file can be staged into several names),
and returns the inner `spec` as JSON. Files without envelope shape pass
through unchanged (raw-spec backward compat).
Threads expected `kind` through every EntityWriter.addEntity caller via the
`KIND` constants on the per-entity command classes (Model, Application,
ToolSet, Schema, Interceptor, Role, Key, Route). validateEntity already had
`kind`.
Issue 2 from the same /dial-uc-debug round (`--dry-run` after subcommand
rejected as `Unknown option`) was already fixed by Cli.3's
ScopeType.INHERIT (1da4a89); the live repro that exposed it was a stale
runner jar from May 5 predating the Cli.3 merge — rebuild fixes it.
Tests:
- modelAddAcceptsManifestEnvelope
- modelAddRejectsWrongKindEnvelope
- modelAddEnvelopeNameMismatchWarnsButProceeds
- modelAddRawSpecBackwardCompat
- modelValidateAcceptsManifestEnvelope
- modelValidateRejectsWrongKindEnvelope
Design anchor: 06 §2.4 (`--from-file accepts two shapes`)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Folds the 2026-05-09 amendment into IMPLEMENTATION.md: §1 MVP-stretch /
out-of-MVP notes; §5.5 MVP-cut callout; six new Track B slice rows
(template DSL, overlays, bundles, ${SECRET:*}, promote --template,
directory-walk apply); removes the 5 promoted bullets from "Deferred
beyond MVP". 4C.6 / 4S.2 explicitly held out per locked decision.
Design anchors: 05 §3, §4, §5.2, §5.3; OQ-18, OQ-19, OQ-29
Tests: no new tests (docs-only)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ceholders)
Adds dial-cli's Phase 4 template engine: extends/includes composition with
deep-merge + cycle detection (mixin-cycle aware), !if/!for via Strategy (i)
pre-parse YAML rewrite, expression evaluator (==,!=,&&,||,!), 7-function
set, ${vars.*}/${params.*}/${entity.*} substitution. ${SECRET:*} passes
through unchanged (4C.4 plugs in resolver). add/validate/apply share
TemplateResolver via the Cli.4 envelope path. Stamped-at-write-time per
OQ-29: pure JsonNode→JsonNode, no live linking.
Design anchors: 05 §3 (3.1–3.5); OQ-18, OQ-29
Tests: cli/src/test/java/com/epam/aidial/cli/TemplateResolutionTest.java
cli/src/test/java/com/epam/aidial/cli/template/{ExpressionEvaluator,TemplateComposer}Test.java
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4C.1 (template DSL) squashed into feature/unified-config as fb11db6. Adds the locked Strategy (i) decision (pre-parse YAML rewrite over custom SnakeYAML Constructor), lists the 8 new template-package classes, SIMPLIFY pass fixes (TemplateContext record, namespace constants, comment trim), REVIEW pass fixes (OR-test tautology, missing invariant guards, nested-\${} clear error, mixin-cycle StackOverflowError fix), and the surfaced --dry-run-needs-env regression held for follow-up per user call.
Recursively walks .yaml/.yml/.json files (hidden paths and unknown extensions skipped, deterministic by-path order). Single-file path unchanged. Closes the cat manifests/*.yaml + temp file workaround used by the Dist.2 newcomer playground. Also renames three 4C.1 test methods to satisfy :cli:checkstyleTest's AbbreviationAsWordInName rule (caught by this slice's build gate). Design anchors: 06 §2.7 Tests: cli/src/test/java/com/epam/aidial/cli/ApplyCommandTest.java Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Status 📋 → ✅ + commit SHA 200f2e3; row backfilled with the eight architect-locked design calls, the loadDirectory IOException|RuntimeException fix, and the 4C.1 :cli:checkstyleTest carve-in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the 4C.1 PlaceholderSubstitutor seam from pass-through to System.getenv
so `${SECRET:openai-key}` resolves at apply/promote time and missing values
fail loud. Adds a single-segment shell-env fallback (design 05 §5.1) so bare
`${ENV_VAR}` placeholders work for CI/CD pipelines. Vault/keychain extension
stays deferred per OQ-19.
Design anchors: 05 §3.1, §5.1; OQ-19
Tests: cli/src/test/.../PlaceholderSubstitutorTest.java (new, 10 cases);
cli/src/test/.../TemplateResolutionTest.applySecretMissingFailsLoud
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Status 📋 → ✅ for 4C.4 (`${SECRET:*}` resolver + `${ENV_VAR}` shell fallback)
on commit 5d118a2. Retrospective folds in the seam-flip detail, REVIEW
false-positive triage (ExpressionEvaluator/ControlFlowExpander resolver
threading), SIMPLIFY WHY-comment, and the test sweep (10 new + 1 flipped).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/deploy-review
|
Adds environment overlays per design 05 §5.2 — kind: <Entity>Overlay with
target + RFC 7396 JSON Merge Patch on spec + optional params override,
plus empty .disable marker files removing matched base entities.
Resolution pipeline sits between ManifestLoader and TemplateResolver,
feeding overlay-overridden params into ${params.X} substitution.
Design anchors: 05 §5.2
Tests: cli/src/test/.../OverlayResolverTest.java,
cli/src/test/.../JsonMergePatchTest.java,
cli/src/test/.../ApplyCommandTest.java (overlay cases)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
astsiapanay
reviewed
May 11, 2026
| } | ||
|
|
||
| /** Returns {@code true} when the caller resolved an authenticated identity (JWT or API key). */ | ||
| public boolean isAuthenticated(ProxyContext context) { |
Collaborator
There was a problem hiding this comment.
The method is useless.
ProxyContext is always available for authenticated users only.
| } | ||
| if (ResourceDescriptor.PUBLIC_BUCKET.equals(bucket)) { | ||
| return operation.isRead() | ||
| ? accessService.isAuthenticated(context) |
Collaborator
There was a problem hiding this comment.
Being authenticated is not enough to have access to config based resources. Authorization must be applied before the use.
| */ | ||
| public boolean isOwnerOf(ProxyContext context, String bucket) { | ||
| try { | ||
| return encryptionService.encrypt(BucketBuilder.buildInitiatorBucket(context)).equals(bucket); |
Collaborator
There was a problem hiding this comment.
There are two things:
- The request might be authenticated via per-request API key.
- It's better to reuse getOwnResourcesAccess
| String entityType = pathMatcher.group(1); | ||
| String bucket = pathMatcher.group("bucket"); | ||
| String path = pathMatcher.group("path"); | ||
| ConfigAuthorizationService authService = new AdminRoleAuthorizationService(proxy.getAccessService()); |
Collaborator
There was a problem hiding this comment.
It's not effective to re-instatiate service instance on every request
| String bucket = pathMatcher.group("bucket"); | ||
| String path = pathMatcher.group("path"); | ||
| ConfigAuthorizationService authService = new AdminRoleAuthorizationService(proxy.getAccessService()); | ||
| return new ConfigResourceController(context, authService, entityType, bucket, path); |
Collaborator
There was a problem hiding this comment.
The common path is to put service instance to Proxy.
| Map.entry("routes", Set.of(PLATFORM_BUCKET)), | ||
| Map.entry("settings", Set.of(PLATFORM_BUCKET))); | ||
|
|
||
| public static boolean isAllowed(String entityType, String bucket) { |
Collaborator
There was a problem hiding this comment.
Nit. The modifier static is not needed if the class is marked with @UtilityClass.
| class EntityBucketBindingTest { | ||
|
|
||
| @Test | ||
| void configResourceRegexMatchesAdminConfigTypes() { |
Collaborator
There was a problem hiding this comment.
The method is out of scope of the class. The scope of responsibility is not right. It should be moved to somewhere else.
|
|
||
| @Data | ||
| public class Auth { | ||
| private String type; |
There was a problem hiding this comment.
I think it should be Enum instead of String
Contributor
Author
|
/deploy-review
|
Contributor
Author
|
/deploy-review
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applicable issues
Description of changes
TBD (WIP)
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.