diff --git a/.changeset/loud-windows-give.md b/.changeset/loud-windows-give.md new file mode 100644 index 0000000000..7e9ea6881d --- /dev/null +++ b/.changeset/loud-windows-give.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Fixed distributed actions not being visible in the scaffolder template actions. + +Depending on the plugin startup order, some of the distributed actions were not being registered correctly, +causing them to be invisible in the scaffolder template actions list. diff --git a/app-config.yaml b/app-config.yaml index eacf9a96aa..b45255d26c 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -65,7 +65,9 @@ backend: - host: example.com - host: '*.mozilla.org' # workingDirectory: /tmp # Use this to configure a working directory for the scaffolder, defaults to the OS temp-dir - + actions: + pluginSources: + - catalog # See README.md in the proxy-backend plugin for information on the configuration format proxy: endpoints: diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts new file mode 100644 index 0000000000..f3a3b15ec9 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts @@ -0,0 +1,278 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConflictError, NotFoundError } from '@backstage/errors'; +import { createTemplateAction } from '@backstage/plugin-scaffolder-node'; +import { DefaultTemplateActionRegistry } from './TemplateActionRegistry'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; +import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha'; + +describe('DefaultTemplateActionRegistry', () => { + let registry: DefaultTemplateActionRegistry; + let mockActionsService: ReturnType; + let mockAuthService: ReturnType; + let mockLogger: ReturnType; + + beforeEach(() => { + mockActionsService = actionsRegistryServiceMock(); + mockAuthService = mockServices.auth(); + mockLogger = mockServices.logger.mock(); + + registry = new DefaultTemplateActionRegistry( + mockActionsService, + mockAuthService, + mockLogger, + ); + }); + + describe('register', () => { + it('should register a template action successfully', () => { + const action = createTemplateAction({ + id: 'test-action', + description: 'Test action', + handler: jest.fn(), + }); + + expect(() => registry.register(action)).not.toThrow(); + }); + + it('should throw ConflictError when registering action with duplicate ID', () => { + const action1 = createTemplateAction({ + id: 'duplicate-action', + description: 'First action', + handler: jest.fn(), + }); + + const action2 = createTemplateAction({ + id: 'duplicate-action', + description: 'Second action', + handler: jest.fn(), + }); + + registry.register(action1); + + expect(() => registry.register(action2)).toThrow(ConflictError); + expect(() => registry.register(action2)).toThrow( + "Template action with ID 'duplicate-action' has already been registered", + ); + }); + }); + + describe('get', () => { + it('should return a registered action', async () => { + const action = createTemplateAction({ + id: 'test-action', + description: 'Test action', + handler: jest.fn(), + }); + + registry.register(action); + const result = await registry.get('test-action'); + + expect(result).toBe(action); + }); + + it('should return action from ActionsService', async () => { + mockActionsService.register({ + name: 'service-action', + title: 'Service Action', + description: 'Service action', + schema: { + input: z => z.object({}), + output: z => z.object({}), + }, + attributes: { + readOnly: true, + destructive: false, + }, + action: async () => ({ output: {} }), + }); + + const result = await registry.get('test:service-action'); + + expect(result.id).toBe('test:service-action'); + expect(result.description).toBe('Service action'); + expect(result.supportsDryRun).toBe(true); + }); + + it('should throw NotFoundError when action is not found', async () => { + await expect(registry.get('non-existent-action')).rejects.toThrow( + NotFoundError, + ); + await expect(registry.get('non-existent-action')).rejects.toThrow( + "Template action with ID 'non-existent-action' is not registered", + ); + }); + + it('should use own service credentials when getting action', async () => { + const spy = jest.spyOn(mockActionsService, 'list'); + + await expect(registry.get('non-existent')).rejects.toThrow(); + + expect(spy).toHaveBeenCalledWith({ + credentials: await mockAuthService.getOwnServiceCredentials(), + }); + }); + }); + + describe('list', () => { + it('should return empty map when no actions are registered', async () => { + const result = await registry.list(); + + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + }); + + it('should return all registered actions', async () => { + const action1 = createTemplateAction({ + id: 'action-1', + description: 'First action', + handler: jest.fn(), + }); + + const action2 = createTemplateAction({ + id: 'action-2', + description: 'Second action', + handler: jest.fn(), + }); + + registry.register(action1); + registry.register(action2); + + const result = await registry.list(); + + expect(result.size).toBe(2); + expect(result.get('action-1')).toBe(action1); + expect(result.get('action-2')).toBe(action2); + }); + + it('should include actions from ActionsService', async () => { + mockActionsService.register({ + name: 'service-action', + title: 'Service Action', + description: 'Service action', + schema: { + input: z => z.object({}), + output: z => z.object({}), + }, + attributes: { + readOnly: true, + destructive: false, + }, + action: async () => ({ output: {} }), + }); + + const result = await registry.list(); + + expect(result.size).toBe(1); + const action = result.get('test:service-action'); + expect(action).toBeDefined(); + expect(action!.id).toBe('test:service-action'); + expect(action!.description).toBe('Service action'); + expect(action!.supportsDryRun).toBe(true); + }); + + it('should prioritize locally registered actions over service actions', async () => { + const localAction = createTemplateAction({ + id: 'test:same-id', + description: 'Local action', + handler: jest.fn(), + }); + + mockActionsService.register({ + name: 'same-id', + title: 'Same ID', + description: 'Service action', + schema: { + input: z => z.object({}), + output: z => z.object({}), + }, + action: async () => ({ output: {} }), + }); + + registry.register(localAction); + const result = await registry.list(); + + expect(result.get('test:same-id')).toBe(localAction); + expect(mockLogger.warn).toHaveBeenCalledWith( + "Template action with ID 'test:same-id' has already been registered, skipping action provided by actions service", + ); + }); + + it('should use provided credentials when listing actions', async () => { + const credentials = mockCredentials.user(); + const spy = jest.spyOn(mockActionsService, 'list'); + + await registry.list({ credentials }); + + expect(spy).toHaveBeenCalledWith({ credentials }); + }); + + it('should use own service credentials when no credentials provided', async () => { + const spy = jest.spyOn(mockActionsService, 'list'); + + await registry.list(); + + expect(spy).toHaveBeenCalledWith({ + credentials: await mockAuthService.getOwnServiceCredentials(), + }); + }); + + it('should set supportsDryRun to false for destructive actions', async () => { + mockActionsService.register({ + name: 'destructive-action', + title: 'Destructive Action', + description: 'Destructive action', + schema: { + input: z => z.object({}), + output: z => z.object({}), + }, + attributes: { + readOnly: false, + destructive: true, + }, + action: async () => ({ output: {} }), + }); + + const result = await registry.list(); + const action = result.get('test:destructive-action'); + + expect(action!.supportsDryRun).toBe(false); + }); + + it('should set supportsDryRun to false for non-readonly actions', async () => { + mockActionsService.register({ + name: 'non-readonly-action', + title: 'Non-readonly Action', + description: 'Non-readonly action', + schema: { + input: z => z.object({}), + output: z => z.object({}), + }, + attributes: { + readOnly: false, + destructive: false, + }, + action: async () => ({ output: {} }), + }); + + const result = await registry.list(); + const action = result.get('test:non-readonly-action'); + + expect(action!.supportsDryRun).toBe(false); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts index d6d7685a21..7bcf120646 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts @@ -16,13 +16,39 @@ import { ConflictError, NotFoundError } from '@backstage/errors'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; +import { ActionsService } from '@backstage/backend-plugin-api/alpha'; +import { + AuthService, + BackstageCredentials, + LoggerService, +} from '@backstage/backend-plugin-api'; +import { isPlainObject } from 'lodash'; +import { Schema } from 'jsonschema'; +import { JsonObject } from '@backstage/types'; + +/** + * @internal + */ +export interface TemplateActionRegistry { + register(action: TemplateAction): void; + get(actionId: string): Promise>; + list(options?: { + credentials?: BackstageCredentials; + }): Promise>>; +} /** * Registry of all registered template actions. */ -export class TemplateActionRegistry { +export class DefaultTemplateActionRegistry implements TemplateActionRegistry { private readonly actions = new Map(); + constructor( + private readonly actionsRegistry: ActionsService, + private readonly auth: AuthService, + private readonly logger: LoggerService, + ) {} + register(action: TemplateAction) { if (this.actions.has(action.id)) { throw new ConflictError( @@ -33,8 +59,8 @@ export class TemplateActionRegistry { this.actions.set(action.id, action); } - get(actionId: string): TemplateAction { - const action = this.actions.get(actionId); + async get(actionId: string): Promise> { + const action = (await this.list()).get(actionId); if (!action) { throw new NotFoundError( `Template action with ID '${actionId}' is not registered. See https://backstage.io/docs/features/software-templates/builtin-actions/ on how to add a new action module.`, @@ -43,7 +69,50 @@ export class TemplateActionRegistry { return action; } - list(): TemplateAction[] { - return [...this.actions.values()]; + async list(options?: { + credentials?: BackstageCredentials; + }): Promise>> { + const ret = new Map(this.actions); + + const { actions } = await this.actionsRegistry.list({ + credentials: + options?.credentials ?? (await this.auth.getOwnServiceCredentials()), + }); + + for (const action of actions) { + if (ret.has(action.id)) { + this.logger.warn( + `Template action with ID '${action.id}' has already been registered, skipping action provided by actions service`, + ); + continue; + } + + ret.set(action.id, { + id: action.id, + description: action.description, + examples: [], + supportsDryRun: + action.attributes?.readOnly === true && + action.attributes?.destructive === false, + handler: async ctx => { + const { output } = await this.actionsRegistry.invoke({ + id: action.id, + input: ctx.input, + credentials: await ctx.getInitiatorCredentials(), + }); + + if (isPlainObject(output)) { + for (const [key, value] of Object.entries(output as JsonObject)) { + ctx.output(key as keyof typeof output, value); + } + } + }, + schema: { + input: action.schema.input as Schema, + output: action.schema.output as Schema, + }, + }); + } + return ret; } } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/index.ts b/plugins/scaffolder-backend/src/scaffolder/actions/index.ts index ad128b19b7..f6bd39397c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/index.ts @@ -16,4 +16,7 @@ export * from './builtin'; -export { TemplateActionRegistry } from './TemplateActionRegistry'; +export { + type TemplateActionRegistry, + DefaultTemplateActionRegistry, +} from './TemplateActionRegistry'; diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts index 7109a6621e..b3f4b5e2cf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts @@ -16,24 +16,43 @@ import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { TemplateActionRegistry } from '../actions'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** @internal */ -export class DecoratedActionsRegistry extends TemplateActionRegistry { +export class DecoratedActionsRegistry implements TemplateActionRegistry { + private readonly innerActions: Map = new Map(); + constructor( private readonly innerRegistry: TemplateActionRegistry, extraActions: Array, ) { - super(); for (const action of extraActions) { - this.register(action); + this.innerActions.set(action.id, action); } } - get(actionId: string): TemplateAction { + async get(actionId: string): Promise { try { - return super.get(actionId); - } catch { - return this.innerRegistry.get(actionId); + return await this.innerRegistry.get(actionId); + } catch (e) { + if (!this.innerActions.has(actionId)) { + throw e; + } + return this.innerActions.get(actionId)!; } } + + async list(options?: { + credentials?: BackstageCredentials; + }): Promise>> { + const inner = await this.innerRegistry.list(options); + return new Map>([ + ...inner, + ...this.innerActions, + ]); + } + + register(action: TemplateAction): void { + this.innerRegistry.register(action); + } } diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts index d384443252..43c5aea40f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts @@ -41,9 +41,9 @@ import fs from 'fs-extra'; import path from 'path'; import { fileURLToPath } from 'url'; import { v4 as uuid } from 'uuid'; -import { TemplateActionRegistry } from '../actions'; import { NunjucksWorkflowRunner } from '../tasks/NunjucksWorkflowRunner'; import { DecoratedActionsRegistry } from './DecoratedActionsRegistry'; +import { TemplateActionRegistry } from '../actions'; interface DryRunInput { spec: TaskSpec; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 8019bd4134..886ddda36f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -15,15 +15,18 @@ */ import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; -import { TemplateActionRegistry } from '../actions'; +import { + DefaultTemplateActionRegistry, + TemplateActionRegistry, +} from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { JsonObject } from '@backstage/types'; import { ConfigReader } from '@backstage/config'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { createTemplateAction, - TaskSecrets, TaskContext, + TaskSecrets, } from '@backstage/plugin-scaffolder-node'; import { UserEntity } from '@backstage/catalog-model'; import { @@ -36,6 +39,7 @@ import { mockCredentials, mockServices, } from '@backstage/backend-test-utils'; +import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha'; describe('NunjucksWorkflowRunner', () => { let actionRegistry: TemplateActionRegistry; @@ -104,7 +108,11 @@ describe('NunjucksWorkflowRunner', () => { // This one is ESM-only stripAnsi = await import('strip-ansi').then(m => m.default); - actionRegistry = new TemplateActionRegistry(); + actionRegistry = new DefaultTemplateActionRegistry( + actionsRegistryServiceMock(), + mockServices.auth(), + mockServices.logger.mock(), + ); fakeActionHandler = jest.fn(); fakeTaskLog = jest.fn(); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index bcae93fb4d..d13fa58a94 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -33,7 +33,7 @@ import { SecureTemplater, SecureTemplateRenderer, } from '../../lib/templating/SecureTemplater'; -import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; +import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts'; import { generateExampleOutput, isTruthy } from './helper'; import { TaskTrackType, WorkflowResponse, WorkflowRunner } from './types'; @@ -61,8 +61,8 @@ import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; import { BackstageLoggerTransport, WinstonLogger } from './logger'; import { convertFiltersToRecord } from '../../util/templating'; import { - CheckpointState, CheckpointContext, + CheckpointState, } from '@backstage/plugin-scaffolder-node/alpha'; type NunjucksWorkflowRunnerOptions = { @@ -245,7 +245,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { return; } const action: TemplateAction = - this.options.actionRegistry.get(step.action); + await this.options.actionRegistry.get(step.action); const { taskLogger } = createStepLogger({ task, step, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index adfdf0b553..7b85dd1270 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -25,7 +25,7 @@ import { TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; import PQueue from 'p-queue'; -import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; +import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts'; import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { WorkflowRunner } from './types'; import { setTimeout } from 'timers/promises'; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 2a0792346d..bcdcedb70d 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -26,7 +26,7 @@ import { resolveSafeChildPath, SchedulerService, } from '@backstage/backend-plugin-api'; -import { Schema } from 'jsonschema'; +import { validate } from 'jsonschema'; import { CompoundEntityRef, Entity, @@ -41,10 +41,10 @@ import { ScmIntegrations } from '@backstage/integration'; import { EventsService } from '@backstage/plugin-events-node'; import { - createConditionAuthorizer, - createPermissionIntegrationRouter, - createConditionTransformer, ConditionTransformer, + createConditionAuthorizer, + createConditionTransformer, + createPermissionIntegrationRouter, } from '@backstage/plugin-permission-node'; import { TaskSpec, @@ -53,11 +53,11 @@ import { } from '@backstage/plugin-scaffolder-common'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION, - RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, RESOURCE_TYPE_SCAFFOLDER_TASK, + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, scaffolderActionPermissions, - scaffolderTaskPermissions, scaffolderPermissions, + scaffolderTaskPermissions, scaffolderTemplatePermissions, taskCancelPermission, taskCreatePermission, @@ -67,6 +67,7 @@ import { } from '@backstage/plugin-scaffolder-common/alpha'; import { TaskBroker, + TaskFilters, TaskStatus, TemplateAction, TemplateFilter, @@ -80,15 +81,14 @@ import { } from '@backstage/plugin-scaffolder-node/alpha'; import { HumanDuration, JsonObject } from '@backstage/types'; import express from 'express'; -import { validate } from 'jsonschema'; import { Duration } from 'luxon'; import { pathToFileURL } from 'url'; import { v4 as uuid } from 'uuid'; import { z } from 'zod'; import { DatabaseTaskStore, + DefaultTemplateActionRegistry, TaskWorker, - TemplateActionRegistry, } from '../scaffolder'; import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; @@ -115,25 +115,22 @@ import { } from '../util/templating'; import { createDefaultFilters } from '../lib/templating/filters/createDefaultFilters'; import { - ScaffolderPermissionRuleInput, - TaskPermissionRuleInput, - isTaskPermissionRuleInput, ActionPermissionRuleInput, isActionPermissionRuleInput, + isTaskPermissionRuleInput, isTemplatePermissionRuleInput, + ScaffolderPermissionRuleInput, + TaskPermissionRuleInput, TemplatePermissionRuleInput, } from './permissions'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { scaffolderActionRules, - scaffolderTemplateRules, scaffolderTaskRules, + scaffolderTemplateRules, } from './rules'; - -import { TaskFilters } from '@backstage/plugin-scaffolder-node'; import { ActionsService } from '@backstage/backend-plugin-api/alpha'; -import { isPlainObject } from 'lodash'; /** * RouterOptions @@ -272,7 +269,11 @@ export async function createRouter( taskBroker = options.taskBroker; } - const actionRegistry = new TemplateActionRegistry(); + const actionRegistry = new DefaultTemplateActionRegistry( + actionsRegistry, + auth, + logger, + ); const templateExtensions = { additionalTemplateFilters: convertFiltersToRecord( @@ -306,48 +307,10 @@ export async function createRouter( workers.push(worker); } - // TODO(blam): it's a little unfortunate that you have to restart the scaffolder - // backend in order to pick these up. We should really just make `ActionsRegistry.get()` async - // and then we can move this logic into the there instead. - // But we can't make those changes until next major. - // Alternatively, we could look at setting up a periodic task that refreshes the actions registry, but - // not feeling that it's worth the complexity. - const { actions: distributedActions } = await actionsRegistry.list({ - credentials: await auth.getOwnServiceCredentials(), - }); - for (const action of actions) { actionRegistry.register(action); } - for (const action of distributedActions) { - actionRegistry.register({ - id: action.id, - description: action.description, - examples: [], - supportsDryRun: - action.attributes?.readOnly === true && - action.attributes?.destructive === false, - handler: async ctx => { - const { output } = await actionsRegistry.invoke({ - id: action.id, - input: ctx.input, - credentials: await ctx.getInitiatorCredentials(), - }); - - if (isPlainObject(output)) { - for (const [key, value] of Object.entries(output as JsonObject)) { - ctx.output(key as keyof typeof output, value); - } - } - }, - schema: { - input: action.schema.input as Schema, - output: action.schema.output as Schema, - }, - }); - } - const launchWorkers = () => workers.forEach(worker => worker.start()); const shutdownWorkers = async () => { @@ -479,16 +442,20 @@ export async function createRouter( eventId: 'action-fetch', request: req, }); + const credentials = await httpAuth.credentials(req); try { - const actionsList = actionRegistry.list().map(action => { - return { - id: action.id, - description: action.description, - examples: action.examples, - schema: action.schema, - }; - }); + const list = await actionRegistry.list({ credentials }); + const actionsList = Array.from(list.values()) + .map(action => { + return { + id: action.id, + description: action.description, + examples: action.examples, + schema: action.schema, + }; + }) + .sort((a, b) => a.id.localeCompare(b.id)); await auditorEvent?.success();