Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide meta repository from users #1115

Merged
merged 4 commits into from
Mar 20, 2025
Merged

Hide meta repository from users #1115

merged 4 commits into from
Mar 20, 2025

Conversation

minwoox
Copy link
Contributor

@minwoox minwoox commented Mar 19, 2025

Motivation:
The meta repository contains mirror and credential configurations, which are now accessed via REST APIs. Since authorization for the meta repository is no longer required, it can be completely hidden from users.

Modifications:

  • Hid meta repositories in all projects.
  • Added a job to remove the meta repository's RepositoryMetadata from the /metadata.json file.

Result:

  • You can no longer see or access the meta repository directly.

@minwoox
Hide meta repository from users
9e33e48 
Motivation:
The `meta` repository contains mirror and credential configurations, which are now accessed via REST APIs.
Since authorization for the `meta` repository is no longer required, it can be completely hidden from users.

Modifications:
- Hid `meta` repositories in all projects.
- Added a job to remove the `meta` repository's `RepositoryMetadata` from the `/metadata.json` file.

Result:
- You can no longer see or access the `meta` repository directly.
@minwoox minwoox added this to the 0.75.0 milestone Mar 19, 2025
trustin
trustin approved these changes Mar 19, 2025
View reviewed changes
server/src/main/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutor.java Outdated
@@ -292,7 +292,8 @@ private CompletableFuture<Void> purgeRepository(PurgeRepositoryCommand c) {
}

private CompletableFuture<CommitResult> push(RepositoryCommand<?> c, boolean normalizing) {
if (c.projectName().equals(INTERNAL_PROJECT_DOGMA) || c.repositoryName().equals(Project.REPO_DOGMA) ||
if (c.projectName().equals(INTERNAL_PROJECT_DOGMA) ||
Project.internalRepos().contains(c.repositoryName()) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace all Project.internalRepos().contains() with Project.isInternalRepo() (a new static method) for brevity? We could introduce some naming rule in the future more easily as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Addressed. 😉

ikhoon
ikhoon approved these changes Mar 19, 2025
View reviewed changes
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍

server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java
@@ -201,8 +198,7 @@ public CompletableFuture<PushResultDto> push(
Author author,
CommitMessageDto commitMessage,
@RequestConverter(ChangesRequestConverter.class) Iterable<Change<?>> changes) {
final User user = AuthUtil.currentUser(ctx);
checkPush(repository.name(), changes, user.isSystemAdmin());
checkMetaRepoPush(repository.name(), changes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this validation as accessing meta is prohibited by the decorator?

Copy link
Contributor Author

@minwoox minwoox Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A system admin can still do the push and I left it because of these lines:
https://github.com/line/centraldogma/pull/1115/files#diff-365d4212ade9fe684cd89a04b979999f32b398703bd3ebbfa99d51d10617c3b4R448
https://github.com/line/centraldogma/pull/1115/files#diff-365d4212ade9fe684cd89a04b979999f32b398703bd3ebbfa99d51d10617c3b4R455

@codecov Codecov
Copy link

codecov bot commented Mar 19, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 90.32258% with 9 lines in your changes missing coverage. Please review.

Project coverage is 70.38%. Comparing base (767eff3) to head (3bed017).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
...dogma/server/internal/api/RepositoryServiceV1.java 33.33% 0 Missing and 2 partials ⚠️
...e/RemovingMetaRepositoryMetadataServicePlugin.java 87.50% 2 Missing ⚠️
...erver/internal/thrift/CentralDogmaServiceImpl.java 33.33% 0 Missing and 2 partials ⚠️
...nternal/api/converter/HttpApiRequestConverter.java 0.00% 0 Missing and 1 partial ⚠️
...internal/replication/ZooKeeperCommandExecutor.java 0.00% 1 Missing ⚠️
...orp/centraldogma/xds/group/v1/XdsGroupService.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #1115      +/-   ##
============================================
+ Coverage     70.07%   70.38%   +0.30%     
- Complexity     4486     4518      +32     
============================================
  Files           453      454       +1     
  Lines         18161    18426     +265     
  Branches       2008     2026      +18     
============================================
+ Hits          12727    12969     +242     
- Misses         4345     4368      +23     
  Partials       1089     1089

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@minwoox minwoox merged commit fcee243 into line:main Mar 20, 2025
12 checks passed
@minwoox minwoox deleted the hide_meta branch March 20, 2025 05:26
@minwoox
Copy link
Contributor Author

minwoox commented Mar 20, 2025

Thanks for reviewing. 😉

@minwoox minwoox mentioned this pull request Mar 20, 2025
Closed
@minwoox minwoox added the defect label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trustin trustin trustin approved these changes

@ikhoon ikhoon ikhoon approved these changes

@jrhee17 jrhee17 jrhee17 approved these changes

Assignees
No one assigned
Labels
defect
Projects
None yet
Milestone
0.75.0
Development

Successfully merging this pull request may close these issues.
4 participants
@minwoox @trustin @ikhoon @jrhee17