feat(core): inject managed by label#533
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe CLI adds a managed-by label option, conditionally labels local resources during configuration loading, forwards the setting through command pipelines, adds coverage, and configures SDK TypeScript project references. ChangesManaged-by label propagation
SDK TypeScript project references
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CLICommand
participant LoadLocalConfigurationTask
participant fillLabels
CLICommand->>LoadLocalConfigurationTask: pass managedByLabel
LoadLocalConfigurationTask->>fillLabels: apply managed-by=adc when enabled
fillLabels-->>LoadLocalConfigurationTask: labeled local resources
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/cli/src/command/utils.spec.ts (1)
259-283: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a case for pre-existing
managed-bylabel overwrite.Current test covers "no labels" and "different label key" merge cases but not a resource that already sets its own
managed-byvalue, which is the scenario most likely to surprise users givenfillLabels's overwrite semantics.🤖 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 `@apps/cli/src/command/utils.spec.ts` around lines 259 - 283, Add a test case alongside the existing fillLabels test that gives a service or consumer an existing managed-by label with a different value, then verifies fillLabels overwrites it with MANAGED_BY_LABEL_VALUE while preserving other labels and resources.apps/cli/src/tasks/load_local.ts (1)
110-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
titleon the new task.Every other task in this pipeline (load, merge, resolve vars, filter, label-selector) declares a
titlefor Listr rendering; this one doesn't, which will show a blank/undefined step in the CLI output.♻️ Proposed fix
{ + title: 'Label resources as managed by ADC', enabled: managedByLabel, task: async () => { fillLabels(ctx.local, { [MANAGED_BY_LABEL_KEY]: MANAGED_BY_LABEL_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 `@apps/cli/src/tasks/load_local.ts` around lines 110 - 117, Add a descriptive title to the task object that invokes fillLabels in the local loading pipeline, matching the title pattern used by the neighboring load, merge, variable-resolution, and filter tasks so Listr renders a named step.
🤖 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.
Nitpick comments:
In `@apps/cli/src/command/utils.spec.ts`:
- Around line 259-283: Add a test case alongside the existing fillLabels test
that gives a service or consumer an existing managed-by label with a different
value, then verifies fillLabels overwrites it with MANAGED_BY_LABEL_VALUE while
preserving other labels and resources.
In `@apps/cli/src/tasks/load_local.ts`:
- Around line 110-117: Add a descriptive title to the task object that invokes
fillLabels in the local loading pipeline, matching the title pattern used by the
neighboring load, merge, variable-resolution, and filter tasks so Listr renders
a named step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b85ab589-457e-4816-b0d3-40671fed5553
📒 Files selected for processing (12)
apps/cli/src/command/diff.command.tsapps/cli/src/command/helper.tsapps/cli/src/command/ingress-sync.command.tsapps/cli/src/command/lint.command.tsapps/cli/src/command/sync.command.tsapps/cli/src/command/typing.d.tsapps/cli/src/command/utils.spec.tsapps/cli/src/command/utils.tsapps/cli/src/command/validate.command.tsapps/cli/src/tasks/load_local.tslibs/sdk/tsconfig.jsonlibs/sdk/tsconfig.spec.json
bzp2010
added
test/api7
juzhiyuan
approved these changes
Jul 13, 2026
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
2 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.
Description
Now inject the following label into the synchronized resource:
This will help administrators using the UI or Admin API understand that the resource was created and is manage lifecycle by ADC. This implies that you should not modify the resource through any process other than ADC-driven GitOps.
This is a behavior that has changed significantly. When you upgrade to a new version of ADC and then perform synchronization, the label values for all resources will be updated.
If you do not need to inject this label, use
--no-managed-by-labelto disable this behavior.If your concern is that this key is the same as the one you're currently using, I suggest using a namespace, such as
acme.com/managed-by. This will clearly indicate that the key belongs to your organization's namespace, ensuring there are no conflicts and providing greater clarity. This concept is similar to Kubernetes.This is enabled only on the CLI; the ADC server does not perform any implicit label injection.
Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
managed-by=adclabel.--no-managed-by-labelto disable automatic labeling.Bug Fixes
Tests
Chores