Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/skip-template-generation-annotation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@openchoreo/backstage-plugin-catalog-backend-module': minor
'@openchoreo/openchoreo-client-node': minor
---

support opting a (Cluster)ComponentType or (Cluster)ResourceType out of auto-generated scaffolder Template (create card) emission via the `openchoreo.dev/skip-template-generation: "true"` annotation. Both the periodic full sync and the event-driven delta path honor it; the delta path actively removes a previously emitted Template when the annotation is added to a live resource. Useful when a type is served by a hand-authored template or is not meant to be user-creatable from the portal.
2 changes: 2 additions & 0 deletions packages/openchoreo-client-node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export {
getAnnotation,
getDisplayName,
getDescription,
SKIP_TEMPLATE_GENERATION_ANNOTATION,
isTemplateGenerationSkipped,
getConditions,
getCondition,
getConditionStatus,
Expand Down
36 changes: 36 additions & 0 deletions packages/openchoreo-client-node/src/resource-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
getAnnotation,
getDisplayName,
getDescription,
isTemplateGenerationSkipped,
SKIP_TEMPLATE_GENERATION_ANNOTATION,
getConditions,
getCondition,
getConditionStatus,
Expand Down Expand Up @@ -176,6 +178,40 @@ describe('getDisplayName', () => {
});
});

describe('isTemplateGenerationSkipped', () => {
it('returns true when the annotation is "true"', () => {
const skipped = {
metadata: {
name: 'api-proxy',
annotations: { [SKIP_TEMPLATE_GENERATION_ANNOTATION]: 'true' },
},
};
expect(isTemplateGenerationSkipped(skipped)).toBe(true);
});

it('returns false for any other value', () => {
const explicitFalse = {
metadata: {
name: 'api-proxy',
annotations: { [SKIP_TEMPLATE_GENERATION_ANNOTATION]: 'false' },
},
};
expect(isTemplateGenerationSkipped(explicitFalse)).toBe(false);
const nonBoolean = {
metadata: {
name: 'api-proxy',
annotations: { [SKIP_TEMPLATE_GENERATION_ANNOTATION]: 'yes' },
},
};
expect(isTemplateGenerationSkipped(nonBoolean)).toBe(false);
});

it('returns false when the annotation is absent', () => {
expect(isTemplateGenerationSkipped(fullResource)).toBe(false);
expect(isTemplateGenerationSkipped(emptyResource)).toBe(false);
});
});

describe('getDescription', () => {
it('returns description annotation', () => {
expect(getDescription(fullResource)).toBe('A test resource');
Expand Down
19 changes: 19 additions & 0 deletions packages/openchoreo-client-node/src/resource-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,25 @@ export function getAnnotation(
return resource.metadata?.annotations?.[key];
}

/**
* Annotation that opts a (Cluster)ComponentType or (Cluster)ResourceType out
* of auto-generated scaffolder Template (create card) emission. Set it to
* `"true"` on the resource to hide its create card, e.g. when the type is
* served by a hand-authored template or is not meant to be user-creatable.
*/
export const SKIP_TEMPLATE_GENERATION_ANNOTATION =
'openchoreo.dev/skip-template-generation';

/**
* Returns true when the resource carries
* {@link SKIP_TEMPLATE_GENERATION_ANNOTATION} set to `"true"`.
*/
export function isTemplateGenerationSkipped(resource: HasMetadata): boolean {
return (
getAnnotation(resource, SKIP_TEMPLATE_GENERATION_ANNOTATION) === 'true'
);
}

// ---------------------------------------------------------------------------
// Display helpers
// ---------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -718,4 +718,121 @@ describe('EventDeltaApplier.handleEvent', () => {
expect.stringContaining('no CatalogService is wired'),
);
});

describe('skip-template-generation annotation', () => {
function ok(data: any) {
return { data, error: undefined, response: { ok: true, status: 200 } };
}

const cctBase = {
metadata: {
name: 'api-proxy',
uid: 'uid-api-proxy',
creationTimestamp: '2025-01-06T10:00:00Z',
annotations: { 'openchoreo.dev/display-name': 'API Proxy' },
},
spec: { workloadType: 'proxy' },
};

it('upserts the ClusterComponentType but actively removes its Template when annotated', async () => {
const applier = newApplier(connection);
const annotated = {
...cctBase,
metadata: {
...cctBase.metadata,
annotations: {
...cctBase.metadata.annotations,
'openchoreo.dev/skip-template-generation': 'true',
},
},
};
mockGET.mockImplementation((path: string) =>
Promise.resolve(
path.includes('/schema')
? ok({ type: 'object', properties: {} })
: ok(annotated),
),
);

await applier.handleEvent(
'ClusterComponentType',
'api-proxy',
undefined,
'updated',
);

// First mutation: upsert of the CCT entity only — no Template.
const upsert = applyMutation.mock.calls[0][0];
expect(upsert.added.map((e: any) => e.entity.kind)).toEqual([
'ClusterComponentType',
]);
// Second mutation: active removal of the (possibly pre-existing)
// Template — the annotation may have been added after emission.
const removal = applyMutation.mock.calls[1][0];
expect(removal.removed.map((r: any) => r.entityRef)).toEqual([
'template:openchoreo-cluster/template-api-proxy',
]);
});

it('still emits the Template when the annotation is absent', async () => {
const applier = newApplier(connection);
mockGET.mockImplementation((path: string) =>
Promise.resolve(
path.includes('/schema')
? ok({ type: 'object', properties: {} })
: ok(cctBase),
),
);

await applier.handleEvent(
'ClusterComponentType',
'api-proxy',
undefined,
'updated',
);

expect(applyMutation).toHaveBeenCalledTimes(1);
const kinds = applyMutation.mock.calls[0][0].added.map(
(e: any) => e.entity.kind,
);
expect(kinds.sort()).toEqual(['ClusterComponentType', 'Template']);
});

it('removes the namespaced Template when an annotated ComponentType is refreshed', async () => {
const applier = newApplier(connection);
const annotatedCt = {
metadata: {
name: 'hidden-ct',
uid: 'uid-hidden-ct',
namespace: 'test-ns',
creationTimestamp: '2025-01-06T10:00:00Z',
annotations: { 'openchoreo.dev/skip-template-generation': 'true' },
},
spec: { workloadType: 'deployment' },
};
mockGET.mockImplementation((path: string) =>
Promise.resolve(
path.includes('/schema')
? ok({ type: 'object', properties: {} })
: ok(annotatedCt),
),
);

await applier.handleEvent(
'ComponentType',
'hidden-ct',
'test-ns',
'updated',
);

const upsert = applyMutation.mock.calls[0][0];
expect(upsert.added.map((e: any) => e.entity.kind)).toEqual([
'ComponentType',
]);
const removal = applyMutation.mock.calls[1][0];
expect(removal.removed.map((r: any) => r.entityRef)).toEqual([
'template:test-ns/template-hidden-ct',
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ import {
getDescription,
getDisplayName,
getName,
isTemplateGenerationSkipped,
SKIP_TEMPLATE_GENERATION_ANNOTATION,
} from '@openchoreo/openchoreo-client-node';
import { CtdToTemplateConverter } from '../converters/CtdToTemplateConverter';
import { RtdToTemplateConverter } from '../converters/RtdToTemplateConverter';
Expand Down Expand Up @@ -1065,6 +1067,18 @@ export class EventDeltaApplier {
ns,
this.translatorContext,
);
if (isTemplateGenerationSkipped(ct)) {
// Annotation may have been added after the Template was emitted, so
// actively remove it rather than just skipping the upsert.
this.logger.debug(
`Template generation skipped for ComponentType ${ns}/${name} (${SKIP_TEMPLATE_GENERATION_ANNOTATION})`,
);
await this.upsertEntities([ctEntity]);
await this.removeEntityRefs([
this.buildEntityRef('template', ns, `template-${name}`),
]);
Comment on lines +1076 to +1079

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use one atomic delta mutation for skip-path updates.

At Line 1076 (and similarly at Lines 1126, 1214, 1256), the skip flow does two separate applyMutation calls (upsert, then remove). If the second call fails or concurrent events interleave, stale Template entities can remain even though skip is set.

Proposed fix
+  private async upsertAndRemoveEntities(
+    addedEntities: Entity[],
+    removedRefs: string[],
+  ): Promise<void> {
+    const connection = this.getConnection();
+    if (!connection || (addedEntities.length === 0 && removedRefs.length === 0)) {
+      return;
+    }
+    await connection.applyMutation({
+      type: 'delta',
+      added: this.toDeferred(addedEntities),
+      removed: removedRefs.map(entityRef => ({
+        entityRef,
+        locationKey: this.locationKey(),
+      })),
+    });
+  }
-      await this.upsertEntities([ctEntity]);
-      await this.removeEntityRefs([
-        this.buildEntityRef('template', ns, `template-${name}`),
-      ]);
+      await this.upsertAndRemoveEntities(
+        [ctEntity],
+        [this.buildEntityRef('template', ns, `template-${name}`)],
+      );

Apply the same pattern in refreshResourceType, refreshClusterComponentType, and refreshClusterResourceType.

Also applies to: 1126-1129, 1214-1222, 1256-1264

🤖 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 `@plugins/catalog-backend-module-openchoreo/src/provider/EventDeltaApplier.ts`
around lines 1076 - 1079, The skip-path update logic at lines 1076-1079 (and
similarly at lines 1126, 1214, and 1256) performs two separate mutation calls -
upsertEntities followed by removeEntityRefs. These should be consolidated into a
single atomic delta mutation to prevent stale Template entities from remaining
if the second call fails or concurrent events interleave. Refactor the upsert
and remove operations in these locations within the refreshResourceType,
refreshClusterComponentType, and refreshClusterResourceType methods to use one
combined atomic delta mutation operation instead of two separate applyMutation
calls.

return;
}
const templateEntity = await this.buildComponentTypeTemplateEntity(
client,
ns,
Expand Down Expand Up @@ -1105,6 +1119,16 @@ export class EventDeltaApplier {
ns,
this.translatorContext,
) as Entity;
if (isTemplateGenerationSkipped(rt)) {
this.logger.debug(
`Template generation skipped for ResourceType ${ns}/${name} (${SKIP_TEMPLATE_GENERATION_ANNOTATION})`,
);
await this.upsertEntities([rtEntity]);
await this.removeEntityRefs([
this.buildEntityRef('template', ns, `template-resource-${name}`),
]);
return;
}
const templateEntity = await this.buildResourceTypeTemplateEntity(
client,
ns,
Expand Down Expand Up @@ -1183,6 +1207,20 @@ export class EventDeltaApplier {
cct,
this.translatorContext,
) as Entity;
if (isTemplateGenerationSkipped(cct)) {
this.logger.debug(
`Template generation skipped for ClusterComponentType ${name} (${SKIP_TEMPLATE_GENERATION_ANNOTATION})`,
);
await this.upsertEntities([cctEntity]);
await this.removeEntityRefs([
this.buildEntityRef(
'template',
'openchoreo-cluster',
`template-${name}`,
),
]);
return;
}
const templateEntity = await this.buildClusterComponentTypeTemplateEntity(
client,
cct,
Expand Down Expand Up @@ -1211,6 +1249,20 @@ export class EventDeltaApplier {
crt,
this.translatorContext,
) as Entity;
if (isTemplateGenerationSkipped(crt)) {
this.logger.debug(
`Template generation skipped for ClusterResourceType ${name} (${SKIP_TEMPLATE_GENERATION_ANNOTATION})`,
);
await this.upsertEntities([crtEntity]);
await this.removeEntityRefs([
this.buildEntityRef(
'template',
'openchoreo-cluster',
`template-resource-${name}`,
),
]);
return;
}
const templateEntity = await this.buildClusterResourceTypeTemplateEntity(
client,
crt,
Expand Down
Loading
Loading