Add deployment-specific metadata properties for projects, repositories and app identities - #1350
Draft
ikhoon wants to merge 1 commit into
Draft
Add deployment-specific metadata properties for projects, repositories and app identities#1350ikhoon wants to merge 1 commit into
ikhoon wants to merge 1 commit into
Conversation
…s and app identities Motivation: Organizations running Central Dogma often need to attach deployment-specific properties, such as the identifier of the owning service, to projects, repositories and app identities. There was no way to declare, validate, store or retrieve such properties without forking the metadata model. Modifications: - Add a `metadataProperties` section to `dogma.json` (and `CentralDogmaBuilder.metadataProperties()`) that declares a JSON Schema per resource type (`project`, `repo` and `appIdentity`). - Accept an optional `properties` object in the creation APIs of projects, repositories and app identities, and validate it against the declared schema with json-schema-validator. A non-conforming request is rejected while undeclared properties are silently dropped, so that a new property can be declared with a rolling restart. - Persist the validated properties in `ProjectMetadata`, `RepositoryMetadata` and `AppIdentity`, and return them via the retrieval APIs. - Add `GET /api/v1/metadataProperties` that exposes the declared schemas so that clients such as the web UI can render input forms. Result: - The server-side extension point of #1346 is now available; the web UI form rendering follows in a stacked PR. - Administrators can require and validate deployment-specific metadata properties at creation time. Nothing changes when `metadataProperties` is not configured.
📝 WalkthroughWalkthroughThis PR adds configurable JSON Schema validation for project, repository, and app-identity metadata properties. It accepts properties through creation APIs, persists them in metadata models, preserves them across lifecycle updates, exposes schemas through an API, and documents the configuration. ChangesMetadata properties
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
server/src/main/java/com/linecorp/centraldogma/server/storage/project/ProjectManager.java (1)
31-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a
defaultmethod to keep this public interface source-compatible.
ProjectManageris in the publiccom.linecorp.centraldogma.server.storage.projectpackage. The new abstract method breaks every existing implementation that does not extendDirectoryBasedStorageManager, including test doubles and third-party managers. A default implementation that delegates to the inherited 4-argcreatepreserves compatibility and mirrors thecreateChildoverload pattern used inDirectoryBasedStorageManager.♻️ Proposed change
- Project create(String name, long creationTimeMillis, Author author, boolean encrypt, - `@Nullable` JsonNode properties); + default Project create(String name, long creationTimeMillis, Author author, boolean encrypt, + `@Nullable` JsonNode properties) { + return create(name, creationTimeMillis, author, encrypt); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/main/java/com/linecorp/centraldogma/server/storage/project/ProjectManager.java` around lines 31 - 35, Update the new properties-aware create method in ProjectManager to be a default method that delegates to the existing four-argument create overload, preserving the prior implementation contract for external implementations. Follow the established overload pattern used by DirectoryBasedStorageManager.createChild.server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java (1)
97-102: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFiltering ignores
patternProperties, so matching input is dropped silently.The filter keys only on names under the top-level
propertieskeyword. If an operator declares bothpropertiesandpatternProperties, every key that matches only a pattern is removed before validation. The request then succeeds, and the data is lost without any error.Consider disabling filtering when the schema also declares
patternPropertiesor a schema-valuedadditionalProperties.♻️ Proposed change
schemas.put(type, schema); final JsonNode propertiesNode = schemaNode.get("properties"); - if (propertiesNode != null && propertiesNode.isObject()) { + // Do not filter when the schema can also accept keys that are not listed under "properties". + final boolean acceptsUnlistedNames = + schemaNode.get("patternProperties") != null || + (schemaNode.get("additionalProperties") != null && + schemaNode.get("additionalProperties").isObject()); + if (propertiesNode != null && propertiesNode.isObject() && !acceptsUnlistedNames) { final ImmutableSet.Builder<String> names = ImmutableSet.builder(); propertiesNode.fieldNames().forEachRemaining(names::add); declaredProperties.put(type, names.build()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java` around lines 97 - 102, Update the declared-property filtering logic in MetadataPropertiesValidator so it does not filter keys solely from properties when the schema declares patternProperties or schema-valued additionalProperties. Detect those schema constructs while building declaredProperties, and disable or bypass filtering for such schemas so pattern-matched and additional schema-valid properties reach validation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@licenses/LICENSE.itu.al20.txt`:
- Line 190: Remove the copied “Copyright 2017 Jayway” attribution from the
license text in licenses/LICENSE.itu.al20.txt, preserving the exact upstream
Apache LICENSE boilerplate from the resolved itu/ethlo/itu artifact.
In `@server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java`:
- Around line 1014-1025: Apply metadataPropertiesValidator validation at the
shared project/repository creation boundary used by non-v1 callers, including
ProjectApiManager.createProject(...), rather than relying only on the V1 service
registrations in CentralDogma. Ensure request properties are validated before
persistence for every creation path, while avoiding duplicate validation in the
existing V1 flow.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java`:
- Around line 104-108: Update AbstractAppIdentity.properties() in
server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java:104-108
and RepositoryMetadata.properties() in
server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java:188-195
to return a deep copy of the stored properties, preserving null when no
properties exist.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java`:
- Around line 1117-1119: Restore the previous public overloads for createToken
and the other affected methods, keeping their original signatures and delegating
to the new properties-aware implementations with null properties. Preserve the
new overloads and ensure existing callers remain source- and binary-compatible.
- Around line 1276-1277: Update the Dogma repository metadata path in
MetadataService to preserve existing properties when changing status: add an
RepositoryMetadata.ofDogma overload accepting properties, and pass
projectMetadata.properties() through the call that currently supplies removal
and properties. Keep the existing no-properties overload behavior for callers
that do not provide properties.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java`:
- Around line 185-186: Update ProjectMetadata.properties() to return a deep
defensive copy of the stored JsonNode instead of the mutable properties field,
ensuring callers and MetadataService transformations cannot mutate validated
state without revalidation.
In `@site/src/sphinx/setup-configuration.rst`:
- Around line 267-274: The metadataProperties filtering logic must preserve
properties when the schema combines root properties with composition keywords
such as $ref or allOf. Update compile/validate handling so composed schemas
either bypass filtering or include the full composed shape, and add regression
coverage for root properties combined with $ref and with allOf.
---
Nitpick comments:
In
`@server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java`:
- Around line 97-102: Update the declared-property filtering logic in
MetadataPropertiesValidator so it does not filter keys solely from properties
when the schema declares patternProperties or schema-valued
additionalProperties. Detect those schema constructs while building
declaredProperties, and disable or bypass filtering for such schemas so
pattern-matched and additional schema-valid properties reach validation
unchanged.
In
`@server/src/main/java/com/linecorp/centraldogma/server/storage/project/ProjectManager.java`:
- Around line 31-35: Update the new properties-aware create method in
ProjectManager to be a default method that delegates to the existing
four-argument create overload, preserving the prior implementation contract for
external implementations. Follow the established overload pattern used by
DirectoryBasedStorageManager.createChild.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 90a5c732-4f16-458a-914a-fe3b0d3c083e
📒 Files selected for processing (51)
NOTICE.txtcommon/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateProjectRequest.javacommon/src/main/java/com/linecorp/centraldogma/internal/api/v1/CreateRepositoryRequest.javadependencies.tomllicenses/LICENSE.itu.al20.txtlicenses/LICENSE.json-schema-validator.al20.txtserver/build.gradleserver/src/main/java/com/linecorp/centraldogma/server/CentralDogma.javaserver/src/main/java/com/linecorp/centraldogma/server/CentralDogmaBuilder.javaserver/src/main/java/com/linecorp/centraldogma/server/CentralDogmaConfig.javaserver/src/main/java/com/linecorp/centraldogma/server/MetadataPropertiesConfig.javaserver/src/main/java/com/linecorp/centraldogma/server/command/Command.javaserver/src/main/java/com/linecorp/centraldogma/server/command/CreateProjectCommand.javaserver/src/main/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutor.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/MetadataPropertiesService.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/ProjectServiceV1.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceUtil.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/storage/DirectoryBasedStorageManager.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProjectManager.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/ProjectApiManager.javaserver/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentity.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityDeserializer.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/CertificateAppIdentity.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.javaserver/src/main/java/com/linecorp/centraldogma/server/metadata/Token.javaserver/src/main/java/com/linecorp/centraldogma/server/storage/project/ProjectManager.javaserver/src/test/java/com/linecorp/centraldogma/server/MetadataPropertiesConfigTest.javaserver/src/test/java/com/linecorp/centraldogma/server/command/CreateProjectCommandTest.javaserver/src/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.javaserver/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceTest.javaserver/src/test/java/com/linecorp/centraldogma/server/internal/api/MetadataPropertiesTest.javaserver/src/test/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidatorTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/AppIdentityDeserializerTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/AppIdentityRegistryTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/CertificateAppIdentityTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTest.javaserver/src/test/java/com/linecorp/centraldogma/server/metadata/TokenTest.javasite/src/sphinx/setup-configuration.rstxds/src/main/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupService.javaxds/src/test/java/com/linecorp/centraldogma/xds/internal/CreatingInternalGroupPlugin.java
| same "printed page" as the copyright notice for easier | ||
| identification within third-party archives. | ||
|
|
||
| Copyright 2017 Jayway |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files | rg '(^licenses/LICENSE.itu\.al20\.txt$|(^|/)NOTICE\.txt$|pom\.xml$|build\.gradle' | head -200 || true
echo
echo "== licenses/LICENSE.itu.al20.txt length and relevant lines =="
wc -l licenses/LICENSE.itu.al20.txt
sed -n '170,205p' licenses/LICENSE.itu.al20.txt
echo
echo "== NOTICE references for itu/Jayway =="
rg -n "itu|Jayway|Morten Haraldsen" NOTICE.txt licenses 2>/dev/null || true
echo
echo "== dependency references for itu/Jayway =="
rg -n "ethlo/itu|itu|Jayway" -S pom.xml build.gradle.xml settings.xml gradle.properties licenses 2>/dev/null || true
echo
echo "== resolve upstream ETHLO ITU LICENSE text =="
curl -fsSL https://raw.githubusercontent.com/ethlo/itu/master/LICENSE -o /tmp/itu-license.txt
wc -l /tmp/itu-license.txt
sed -n '1,240p' /tmp/itu-license.txt
echo
echo "== parse Java-style copyright/comment blocks from licenses/LICENSE.itu.al20.txt =="
python3 - <<'PY'
from pathlib import Path
import re
text=Path('licenses/LICENSE.itu.al20.txt').read_text()
for m in re.finditer(r'/(?:\*|\*{1}( |\*{1})|\*\*).*?\*/', text, re.S):
s=m.start().count('\n')+1
if 'Jayway' in m.group(0) or 'itu' in m.group(0).lower():
print(f'-- block {s} --')
print('\n'.join(f'{s+i}: {line}' for i,line in enumerate(m.group(0).splitlines())))
PYRepository: line/centraldogma
Length of output: 27395
Replace the copied itu license attribution.
licenses/LICENSE.itu.al20.txt is listed in NOTICE.txt for dependency itu/ethlo/itu, but line 190 copies Copyright 2017 Jayway from the Jayway JsonPath license. Use the exact license text from the resolved itu artifact, whose upstream Apache LICENSE boilerplate does not include this copyright.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@licenses/LICENSE.itu.al20.txt` at line 190, Remove the copied “Copyright 2017
Jayway” attribution from the license text in licenses/LICENSE.itu.al20.txt,
preserving the exact upstream Apache LICENSE boilerplate from the resolved
itu/ethlo/itu artifact.
Source: MCP tools
Comment on lines
+1014
to
+1025
| final MetadataPropertiesValidator metadataPropertiesValidator = | ||
| new MetadataPropertiesValidator(cfg.metadataProperties()); | ||
| final ContextPathServicesBuilder apiV1ServiceBuilder = sb.contextPath(API_V1_PATH_PREFIX); | ||
| apiV1ServiceBuilder | ||
| .annotatedService(new ServerStatusService(executor, statusManager)) | ||
| .annotatedService(new ProjectServiceV1(projectApiManager, executor)) | ||
| .annotatedService(new RepositoryServiceV1(executor, mds, encryptionStorageManager)) | ||
| .annotatedService(new ProjectServiceV1(projectApiManager, executor, | ||
| metadataPropertiesValidator)) | ||
| .annotatedService(new RepositoryServiceV1(executor, mds, encryptionStorageManager, | ||
| metadataPropertiesValidator)) | ||
| .annotatedService(new CredentialServiceV1(projectApiManager, executor)) | ||
| .annotatedService(new VariableServiceV1(pm, executor)); | ||
| .annotatedService(new VariableServiceV1(pm, executor)) | ||
| .annotatedService(new MetadataPropertiesService(cfg.metadataProperties())); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 \
'\b(createProject|createRepository)\s*\(|new CentralDogmaServiceImpl|new RepositoryService|configureThriftService' \
server/src/main/javaRepository: line/centraldogma
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ProjectApiManager constructor/signature =="
rg -n -C 8 'class ProjectApiManager|ProjectApiManager\(' server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/ProjectApiManager.java
echo
echo "== ProjectApiManager createProject/createRepository relevant methods =="
sed -n '1,220p' server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/ProjectApiManager.java | cat -n
echo
echo "== GitHttpService relevant methods =="
rg -n -C 10 'class GitHttpService|createProject|createRepository|createRepository\(' server/src/main/java/com/linecorp/centraldogma/server/internal/api/v1/GitHttpService.java
echo
echo "== RepositoryService V0 methods =="
rg -n -C 10 'class RepositoryService|createProject|createRepository' server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryService.java
fd -a 'RepositoryService.java' server/src/main/java | while read -r f; do echo "-- $f"; rg -n -C 8 'class RepositoryService|createProject|createRepository' "$f"; done
echo
echo "== CentralDogmaServiceImpl relevant methods =="
sed -n '120,190p' server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java | cat -n
echo
echo "== MetadataPropertiesValidator definitions/usages =="
rg -n -C 8 'class MetadataPropertiesValidator|MetadataPropertiesValidator|validateProperties|properties()' server/src/main/javaRepository: line/centraldogma
Length of output: 12641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate candidate files =="
fd -a 'GitHttpService|RepositoryService|ThriftServiceConfigurator|CentralDogmaServiceImpl|MetadataPropertiesValidator' server/src/main/java
echo
echo "== ProjectApiManager createRepository call path =="
rg -n -C 12 'public CompletableFuture<Void> createRepository|createRepository\(' server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/ProjectApiManager.java || true
echo
echo "== GitHttpService method implementation =="
fd -a 'GitHttpService.java' server/src/main/java | while read -r f; do
echo "-- $f"
rg -n -C 10 'createProject|createRepository|repository|project' "$f" || true
done
echo
echo "== All project/repository create methods in server main sources (non-command) =="
rg -n -C 8 'createProject\(|createRepository\(' server/src/main/java/com/linecorp/centraldogma/server || true
echo
echo "== RepositoryService definitions =="
fd -a 'RepositoryService.java' server/src/main/java | while read -r f; do
echo "-- $f"
rg -n -C 10 'createProject|createRepository' "$f" || true
doneRepository: line/centraldogma
Length of output: 50373
Route non-v1 creation paths through the metadata validator.
The metadataPropertiesValidator is only attached to HTTP v1 services. Thrift project creation calls metadataPropertiesValidator.validate(...) after ProjectServiceV1.createProject(...), so ProjectApiManager.createProject(...) from Thrift sends request.properties() validated by the V1 service only if it is the same call path; separate non-v1 callers need the same validation at the shared creation boundary, otherwise projects or repositories can be persisted without applying the configured metadata schema.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java`
around lines 1014 - 1025, Apply metadataPropertiesValidator validation at the
shared project/repository creation boundary used by non-v1 callers, including
ProjectApiManager.createProject(...), rather than relying only on the V1 service
registrations in CentralDogma. Ensure request properties are validated before
persistence for every creation path, while avoiding duplicate validation in the
existing V1 flow.
Comment on lines
+104
to
+108
| @Nullable | ||
| @Override | ||
| public JsonNode properties() { | ||
| return properties; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not expose mutable stored JSON properties.
Both constructors deep-copy the input, but both accessors return the stored mutable JsonNode. A caller can change metadata without a revision and can invalidate equals() or hashCode() results. Return a deep copy from each accessor.
server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java#L104-L108: returnproperties != null ? properties.deepCopy() : null.server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java#L188-L195: returnproperties != null ? properties.deepCopy() : null.
📍 Affects 2 files
server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java#L104-L108(this comment)server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java#L188-L195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java`
around lines 104 - 108, Update AbstractAppIdentity.properties() in
server/src/main/java/com/linecorp/centraldogma/server/metadata/AbstractAppIdentity.java:104-108
and RepositoryMetadata.properties() in
server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadata.java:188-195
to return a deep copy of the stored properties, preserving null when no
properties exist.
Comment on lines
+1117
to
+1119
| public CompletableFuture<Revision> createToken(Author author, String appId, boolean isSystemAdmin, | ||
| @Nullable JsonNode properties) { | ||
| return appIdentityService.createToken(author, appId, isSystemAdmin, properties); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Retain the previous public overloads.
Lines 1117-1119, 1133-1134, and 1311-1314 replace existing method descriptors. Existing callers of the three previous overloads will fail to compile or link. Keep the old overloads and delegate them with null properties.
Proposed compatibility overloads
+public CompletableFuture<Revision> createToken(Author author, String appId, boolean isSystemAdmin) {
+ return createToken(author, appId, isSystemAdmin, null);
+}
+
+public CompletableFuture<Revision> createToken(Author author, String appId, String secret,
+ boolean isSystemAdmin) {
+ return createToken(author, appId, secret, isSystemAdmin, null);
+}
+
+public CompletableFuture<Revision> createCertificate(Author author, String appId, String certificateId,
+ boolean isSystemAdmin) {
+ return createCertificate(author, appId, certificateId, isSystemAdmin, null);
+}Also applies to: 1133-1134, 1311-1314
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java`
around lines 1117 - 1119, Restore the previous public overloads for createToken
and the other affected methods, keeping their original signatures and delegating
to the new properties-aware implementations with null properties. Preserve the
new overloads and ensure existing callers remain source- and binary-compatible.
Comment on lines
+1276
to
+1277
| projectMetadata.removal(), | ||
| projectMetadata.properties()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Preserve properties of the Dogma repository.
When newRepoName is dogma, Line 1262 creates metadata with RepositoryMetadata.ofDogma(repositoryStatus). That factory sets properties to null. Updating only the status therefore deletes existing Dogma repository properties. Add an ofDogma overload that accepts properties and forward the existing value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java`
around lines 1276 - 1277, Update the Dogma repository metadata path in
MetadataService to preserve existing properties when changing status: add an
RepositoryMetadata.ofDogma overload accepting properties, and pass
projectMetadata.properties() through the call that currently supplies removal
and properties. Keep the existing no-properties overload behavior for callers
that do not provide properties.
Comment on lines
+185
to
+186
| public JsonNode properties() { | ||
| return properties; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return a defensive copy from properties().
properties() returns the mutable stored JsonNode. A caller can change validated properties after construction. Later MetadataService transforms can then persist the changed value without schema validation.
Proposed fix
public JsonNode properties() {
- return properties;
+ return properties != null ? properties.deepCopy() : null;
}Based on provided context, MetadataService reconstructs ProjectMetadata from this accessor during metadata transforms.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public JsonNode properties() { | |
| return properties; | |
| public JsonNode properties() { | |
| return properties != null ? properties.deepCopy() : null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java`
around lines 185 - 186, Update ProjectMetadata.properties() to return a deep
defensive copy of the stored JsonNode instead of the mutable properties field,
ensuring callers and MetadataService transformations cannot mutate validated
state without revalidation.
Comment on lines
+267
to
+274
| - ``metadataProperties`` | ||
|
|
||
| - the additional metadata properties of projects, repositories and app identities. Each field is a | ||
| `JSON Schema <https://json-schema.org/>`_ that the ``properties`` of the corresponding resource must | ||
| conform to at creation time. Properties that are not declared in the schema's top-level | ||
| ``properties`` keyword are silently dropped rather than rejected, so that a new property can be | ||
| declared with a rolling restart. If the schema declares its shape in another way (e.g. ``$ref`` or | ||
| ``allOf``), nothing is dropped and the whole object is validated as is. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 \
'declaredProperties|propertiesNode|allOf|\$ref' \
server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java \
server/src/test/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidatorTest.javaRepository: line/centraldogma
Length of output: 19336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java \
server/src/test/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidatorTest.java
echo '--- MetadataPropertiesValidator.java 82-170 ---'
sed -n '82,170p' server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java | cat -n
echo '--- MetadataPropertiesValidatorTest.java relevant sections ---'
sed -n '1,140p' server/src/test/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidatorTest.java | cat -n
echo '--- allOf/ref references in validators/tests ---'
rg -n -C 4 '\$ref|allOf|anyOf|oneOf|composed|Drop|drops|kept|declared' server/src/main/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidator.java server/src/test/java/com/linecorp/centraldogma/server/internal/metadata/MetadataPropertiesValidatorTest.javaRepository: line/centraldogma
Length of output: 30914
Align filtering with schema composition handling.
compile() adds top-level properties names even when the schema also has $ref or allOf, but validate() then drops every property not in that set. This contradicts the documented behavior: shapes declared through $ref or allOf should not drop properties before validation. Add a regression for root properties plus $ref or allOf, then either disable filtering for composed schemas or merge declared names with the actual schema shape before filtering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/src/sphinx/setup-configuration.rst` around lines 267 - 274, The
metadataProperties filtering logic must preserve properties when the schema
combines root properties with composition keywords such as $ref or allOf. Update
compile/validate handling so composed schemas either bypass filtering or include
the full composed shape, and add regression coverage for root properties
combined with $ref and with allOf.
ikhoon
marked this pull request as draft
August 6, 2026 05:41
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
1 participant
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.
Motivation:
Organizations running Central Dogma often need to attach deployment-specific
properties, such as the identifier of the owning service, to projects,
repositories and app identities. There was no way to declare, validate, store
or retrieve such properties without forking the metadata model.
Modifications:
metadataPropertiessection todogma.json(andCentralDogmaBuilder.metadataProperties()) that declares a JSON Schema perresource type (
project,repoandappIdentity).propertiesobject in the creation APIs of projects,repositories and app identities, and validate it against the declared schema
with json-schema-validator. A non-conforming request is rejected while
undeclared properties are silently dropped, so that a new property can be
declared with a rolling restart.
ProjectMetadata,RepositoryMetadataand
AppIdentity, and return them via the retrieval APIs.GET /api/v1/metadataPropertiesthat exposes the declared schemas sothat clients such as the web UI can render input forms.
Result:
rendering follows in the stacked PR.
properties at creation time. Nothing changes when
metadataPropertiesisnot configured.