Skip to content

Commit

Permalink
would this work ?
Browse files Browse the repository at this point in the history
  • Loading branch information
sobolk committed Sep 17, 2024
1 parent ac67ec4 commit 9816f17
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 27 deletions.
7 changes: 3 additions & 4 deletions packages/backend/src/engine/backend-secret/backend_secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ export class CfnTokenBackendSecret implements BackendSecret {
constructor(
private readonly secretName: string,
private readonly secretResourceFactory: BackendSecretFetcherFactory
) {
BackendSecretFetcherFactory.registerSecret(secretName);
}
) {}
/**
* Get a reference to the value within a CDK scope.
*/
Expand All @@ -30,7 +28,8 @@ export class CfnTokenBackendSecret implements BackendSecret {
): SecretValue => {
const secretResource = this.secretResourceFactory.getOrCreate(
scope,
backendIdentifier
backendIdentifier,
this.secretName
);

const val = secretResource.getAttString(`${this.secretName}`);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Construct } from 'constructs';
import { BackendSecretFetcherProviderFactory } from './backend_secret_fetcher_provider_factory.js';
import { CustomResource } from 'aws-cdk-lib';
import { CustomResource, Lazy } from 'aws-cdk-lib';
import { BackendIdentifier } from '@aws-amplify/plugin-types';
import { SecretResourceProps } from './lambda/backend_secret_fetcher_types.js';

Expand All @@ -18,7 +18,7 @@ const SECRET_RESOURCE_TYPE = `Custom::SecretFetcherResource`;
* The factory to create backend secret-fetcher resource.
*/
export class BackendSecretFetcherFactory {
static secretNames: Set<string> = new Set<string>();
private readonly secretNames: Set<string> = new Set<string>();

/**
* Creates a backend secret-fetcher resource factory.
Expand All @@ -27,29 +27,16 @@ export class BackendSecretFetcherFactory {
private readonly secretProviderFactory: BackendSecretFetcherProviderFactory
) {}

/**
* Register secrets that to be fetched by the BackendSecretFetcher custom resource.\
* @param secretName the name of the secret
*/
static registerSecret = (secretName: string): void => {
BackendSecretFetcherFactory.secretNames.add(secretName);
};

/**
* Clear registered secrets that will be fetched by the BackendSecretFetcher custom resource.
*/
static clearRegisteredSecrets = (): void => {
BackendSecretFetcherFactory.secretNames.clear();
};

/**
* Returns a resource if it exists in the input scope. Otherwise,
* creates a new one.
*/
getOrCreate = (
scope: Construct,
backendIdentifier: BackendIdentifier
backendIdentifier: BackendIdentifier,
secretName: string
): CustomResource => {
this.secretNames.add(secretName);

This comment has been minimized.

Copy link
@awsluja

awsluja Sep 17, 2024

Contributor

this is effectively the same thing as using a static, but now it more unclear that this is global, because to know it's global requires knowing we initialize the BackendSecretFetcherFactory exactly once in secret.ts

This comment has been minimized.

Copy link
@sobolk

sobolk Sep 17, 2024

Author Member

On the other hand. We don't have to craft special API to reset state just for testing and we keep design open to re-scoping later should we change our mind.

Some say that static code should be avoided unless there's good reason to have it.
https://medium.com/att-israel/should-you-avoid-using-static-ae4b58ca1de5
https://www.geeksforgeeks.org/why-you-should-prefer-singleton-pattern-over-static-methods/

(from my experience this is a good advice for any language that supports OOP).

const secretResourceId = `SecretFetcherResource`;
const existingResource = scope.node.tryFindChild(
secretResourceId
Expand All @@ -75,7 +62,9 @@ export class BackendSecretFetcherFactory {
namespace: backendIdentifier.namespace,
name: backendIdentifier.name,
type: backendIdentifier.type,
secretNames: Array.from(BackendSecretFetcherFactory.secretNames),
secretNames: Lazy.list({
produce: () => Array.from(this.secretNames),
}),
};

return new CustomResource(scope, secretResourceId, {
Expand Down
9 changes: 5 additions & 4 deletions packages/backend/src/secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { CfnTokenBackendSecret } from './engine/backend-secret/backend_secret.js
import { BackendSecretFetcherProviderFactory } from './engine/backend-secret/backend_secret_fetcher_provider_factory.js';
import { BackendSecretFetcherFactory } from './engine/backend-secret/backend_secret_fetcher_factory.js';

const secretProviderFactory = new BackendSecretFetcherProviderFactory();
const secretResourceFactory = new BackendSecretFetcherFactory(
secretProviderFactory
);

/**
* Use a secret from AWS Systems Manager (SSM) Parameter Store
* @see https://docs.amplify.aws/gen2/deploy-and-host/fullstack-branching/secrets-and-vars/
Expand All @@ -27,9 +32,5 @@ import { BackendSecretFetcherFactory } from './engine/backend-secret/backend_sec
* ```
*/
export const secret = (name: string): BackendSecret => {
const secretProviderFactory = new BackendSecretFetcherProviderFactory();
const secretResourceFactory = new BackendSecretFetcherFactory(
secretProviderFactory
);
return new CfnTokenBackendSecret(name, secretResourceFactory);
};

0 comments on commit 9816f17

Please sign in to comment.