Allow secrets to be used in build args while registering

src/commands/deploy.ts

@@ -141,25 +141,6 @@ export default class Deploy extends DeployCommand {
       deprecated: true,
       hidden: true,
     }),
-    'secret-file': Flags.string({
-      description: 'Path of secrets file',
-      multiple: true,
-      default: [],
-    }),
-    secrets: Flags.string({
-      description: `Please use --secret-file.`,
-      multiple: true,
-      hidden: true,
-      deprecated: {
-        to: 'secret-file',
-      },
-    }),
-    secret: Flags.string({
-      char: 's',
-      description: 'An individual secret key and value in the form SECRET_KEY=SECRET_VALUE',
-      multiple: true,
-      default: [],
-    }),
     values: Flags.string({
       char: 'v',
       hidden: true,

src/commands/register.ts

@@ -25,6 +25,7 @@ import PluginManager from '../common/plugins/plugin-manager';
 import BuildPackUtils from '../common/utils/buildpack';
 import { transformVolumeSpec } from '../dependency-manager/spec/transform/common-transform';
 import { IF_EXPRESSION_REGEX } from '../dependency-manager/spec/utils/interpolation';
+import DeployUtils from '../common/utils/deploy.utils';
 
 tmp.setGracefulCleanup();
 
@@ -46,6 +47,25 @@ export const SHARED_REGISTER_FLAGS = {
     description: 'Directory to write build cache to. Do not use in Github Actions: https://docs.architect.io/deployments/automated-previews/#caching-between-workflow-runs',
     sensitive: false,
   }),
+  'secret-file': Flags.string({
+    description: 'Path of secrets file',
+    multiple: true,
+    default: [],
+  }),
+  secrets: Flags.string({
+    description: `Please use --secret-file.`,
+    multiple: true,
+    hidden: true,
+    deprecated: {
+      to: 'secret-file',
+    },
+  }),
+  secret: Flags.string({
+    char: 's',
+    description: 'An individual secret key and value in the form SECRET_KEY=SECRET_VALUE',
+    multiple: true,
+    default: [],
+  }),
 };
 
 interface ImageRefOutput {
@@ -197,7 +217,21 @@ export default class ComponentRegister extends BaseCommand {
     const dependency_manager = new LocalDependencyManager(this.app.api, selected_account.name);
     dependency_manager.environment = 'production';
 
-    const graph = await dependency_manager.getGraph([instanceToInstance(component_spec)], undefined, { interpolate: false, validate: false });
+    const all_secret_file_values = [...(flags['secret-file'] || []), ...(flags.secrets || [])];
+    const component_secrets = DeployUtils.getComponentSecrets(flags.secret, all_secret_file_values);
+
+    // Default graph options
+    const graph_options = { interpolate: false, validate: false, interpolate_build_args: false };
+    // If the spec contains a service that has build args, set validate/interpolate_build_args
+    // so that secrets are validated and enforce any required secrets are provided via --secret-file.
+    for (const service of Object.values(component_spec.services || {})) {
+      if (service.build && service.build.args) {
+        graph_options.validate = true;
+        graph_options.interpolate_build_args = true;
+      }
+    }
+
+    const graph = await dependency_manager.getGraph([instanceToInstance(component_spec)], component_secrets, graph_options);
     // Tmp fix to register host overrides
     for (const node of graph.nodes.filter(n => n instanceof ServiceNode) as ServiceNode[]) {
       for (const interface_config of Object.values(node.interfaces)) {

src/common/utils/deploy.utils.ts

@@ -64,6 +64,10 @@ export default class DeployUtils {
     return flags;
   }
 
+  /**
+   * Combine secrets from the secret flag, a secret file, and optionally an existing SecretsDict
+   * into a single SecretsDict.
+   */
   static getComponentSecrets(individual_secrets: string[], secrets_file: string[], env_secrets?: SecretsDict): SecretsDict {
     let component_secrets: SecretsDict = env_secrets ? env_secrets : {};
 

src/dependency-manager/graph/type.ts

@@ -1,4 +1,5 @@
 export interface GraphOptions {
   interpolate?: boolean;
   validate?: boolean;
+  interpolate_build_args?: boolean;
 }

src/dependency-manager/manager.ts

@@ -289,7 +289,7 @@ export default abstract class DependencyManager {
       ...secrets_dict,
     };
 
-    if (options.interpolate && options.validate) {
+    if ((options.interpolate || options.interpolate_build_args) && options.validate) {
       secrets.validateComponentSpec(component_spec);
     }
 
@@ -299,6 +299,13 @@ export default abstract class DependencyManager {
 
       // Replace conditionals
       component_spec = interpolateObject(component_spec, context, { keys: true, values: false, file: component_spec.metadata.file });
+    } else if (options.interpolate_build_args) {
+      // Only interpolate the build args block - used when registering a component to interpolate secret values.
+      for (const service of Object.values(component_spec.services || {})) {
+        if (service.build && service.build.args) {
+          service.build.args = interpolateObject(service.build.args, context, { keys: false, values: true, file: component_spec.metadata.file });
+        }
+      }
     }
 
     const component_config = transformComponentSpec(component_spec);

src/dependency-manager/secrets/secrets.ts

@@ -111,6 +111,10 @@ export class Secrets {
     }
   }
 
+  /**
+   * Validates that all required secrets are provided. If any required secrets are missing,
+   * raises a ValidationErrors error.
+   */
   validateComponentSpec(component_spec: ComponentSpec): void {
     const secrets_dict = this.getSecretsForComponentSpec(component_spec, true);
 

src/dependency-manager/utils/rules.ts

@@ -30,9 +30,17 @@ abstract class InterpolationRule {
 }
 
 class BuildInterpolationRule extends InterpolationRule {
-  protected checkKey(key: string) {
-    const split = key.split('.').filter(key => !key.startsWith('${{'));
-    return (split[0] === 'services' || split[0] === 'tasks') && split[2] === 'build';
+  /**
+   * Returns true if the interpolation is inside of a build block
+   * and the interpolation is not a secret value.
+   */
+  protected checkPathAndKey(path: string, key?: string) {
+    const split = path.split('.').filter(path => !path.startsWith('${{'));
+    const is_build_block = (split[0] === 'services' || split[0] === 'tasks') && split[2] === 'build';
+    if (is_build_block && key) {
+      return !key.startsWith('secrets');
+    }
+    return is_build_block;
   }
 
   check(context_map: ContextMap, context_key: string): string | undefined {
@@ -42,15 +50,14 @@ class BuildInterpolationRule extends InterpolationRule {
       return;
     }
 
-    // Check if the interpolation is inside of a build block
-    if (this.checkKey(context_map._path)) {
-      return `Cannot use \${{ ${context_key} }} inside a build block. The build block is immutable. Use architect dev --arg KEY=VALUE for build args.`;
+    if (this.checkPathAndKey(context_map._path, context_key)) {
+      return `Cannot use \${{ ${context_key} }} inside a build block. The build block is immutable. Use architect dev --arg KEY=VALUE or use secrets for build args.`;
     }
 
     // Check if the interpolation is around a build block
-    const maybe_child_key = Object.keys(context_map._obj_map).find(key => key.startsWith(`${context_map._path}.`) && this.checkKey(key));
+    const maybe_child_key = Object.keys(context_map._obj_map).find(key => key.startsWith(`${context_map._path}.`) && this.checkPathAndKey(key));
     if (maybe_child_key) {
-      return `Cannot use \${{ ${context_key} }} around a build block. The build block is immutable. Use architect dev --arg KEY=VALUE for build args.`;
+      return `Cannot use \${{ ${context_key} }} around a build block. The build block is immutable. Use architect dev --arg KEY=VALUE or use secrets for build args.`;
     }
   }
 }

test/commands/register.test.ts

@@ -15,7 +15,6 @@ import PluginManager from '../../src/common/plugins/plugin-manager';
 import BuildPackUtils from '../../src/common/utils/buildpack';
 import { IF_EXPRESSION_REGEX } from '../../src/dependency-manager/spec/utils/interpolation';
 import { getMockComponentContextPath, getMockComponentFilePath, MockArchitectApi, ReplyCallback } from '../utils/mocks';
-import { Body, ReplyBody } from 'nock/types';
 
 describe('register', function () {
   const mock_account_response = {
@@ -60,11 +59,11 @@ describe('register', function () {
         const text_file = fs.readFileSync('test/mocks/superset/filedata.txt');
         expect(body.config.services['stateful-api'].environment.FILE_DATA).to.eq(text_file.toString().trim());
         return body;
-      }
+      },
     })
     .getTests()
     .stub(ComponentRegister.prototype, 'uploadVolume', sinon.stub().returns({}))
-    .command(['register', 'test/mocks/superset/architect.yml', '-a', 'examples'])
+    .command(['register', 'test/mocks/superset/architect.yml', '-a', 'examples', '-s', 'param_unset=foo'])
     .it('test file: replacement', ctx => {
       expect(ctx.stdout).to.contain('Successfully registered component');
     });
@@ -120,7 +119,7 @@ describe('register', function () {
     .stub(DockerComposeUtils, 'writeCompose', sinon.stub())
     .stub(fs, 'move', sinon.stub())
     .stub(ComponentRegister.prototype, 'uploadVolume', sinon.stub().returns({}))
-    .command(['register', 'test/mocks/superset/architect.yml', '-t', '1.0.0', '-a', 'examples'])
+    .command(['register', 'test/mocks/superset/architect.yml', '-t', '1.0.0', '-a', 'examples', '-s', 'param_unset=foo'])
     .it('register superset', async ctx => {
       expect(ctx.stdout).to.contain('Successfully registered component');
       /*
@@ -160,15 +159,15 @@ describe('register', function () {
     .architectRegistryHeadRequest('/v2/examples/superset.services.stateful-frontend/manifests/1.0.0')
     .architectRegistryHeadRequest('/v2/examples/superset.tasks.curler-build/manifests/1.0.0')
     .getTests()
-    .command(['register', 'test/mocks/superset/architect.yml', '-t', '1.0.0', '-a', 'examples'])
+    .command(['register', 'test/mocks/superset/architect.yml', '-t', '1.0.0', '-a', 'examples', '-s', 'param_unset=foo'])
     .it('it reports to the user that the superset was registered successfully', ctx => {
       expect(ctx.stdout).to.contain('Successfully registered component');
     });
 
   new MockArchitectApi()
     .getAccount(mock_account_response)
     .architectRegistryHeadRequest()
-    .getEnvironment(mock_account_response, { name: 'test-env'})
+    .getEnvironment(mock_account_response, { name: 'test-env' })
     .registerComponentDigest(mock_account_response, { body:
       (body) => {
         expect(body.tag).to.eq('architect.environment.test-env');
@@ -447,7 +446,7 @@ describe('register', function () {
       build: () => { },
     }))
     .stub(DockerBuildXUtils, 'dockerBuildX', sinon.stub())
-    .command(['register', 'test/mocks/buildpack/buildpack-architect.yml', '-t', '1.0.0', '-a', 'examples'])
+    .command(['register', 'test/mocks/buildpack/buildpack-architect.yml', '-t', '1.0.0', '-a', 'examples', '-s', 'param_unset=foo'])
     .it('register with buildpack set to true override Dockerfile', ctx => {
       expect(ctx.stderr).to.contain('Registering component hello-world:1.0.0 with Architect Cloud...... done\n');
       expect(ctx.stdout).to.contain('Successfully registered component');
@@ -470,7 +469,7 @@ describe('register', function () {
     }))
     .stub(DockerHelper, 'composeVersion', sinon.stub().returns(true))
     .stub(DockerHelper, 'buildXVersion', sinon.stub().returns(true))
-    .command(['register', 'test/mocks/buildpack/buildpack-dockerfile-architect.yml', '-t', '1.0.0', '-a', 'examples'])
+    .command(['register', 'test/mocks/buildpack/buildpack-dockerfile-architect.yml', '-t', '1.0.0', '-a', 'examples', '-s', 'param_unset=foo'])
     .it('register with buildpack and dockerfile services', ctx => {
       const buildpack = BuildPackUtils.build as sinon.SinonStub;
       expect(buildpack.args.toString()).to.equal(`${path.normalize('test/plugins')},hello-world--buildpack-api,,${path.join(path.resolve('test/integration'), './hello-world/')}`);
@@ -487,7 +486,7 @@ describe('register', function () {
     .getTests()
     .stub(DockerBuildXUtils, 'dockerBuildX', sinon.stub())
     .stub(DockerUtils, 'doesDockerfileExist', sinon.stub().callsFake(DockerUtils.doesDockerfileExist)) // override global stub
-    .command(['register', 'test/mocks/register/nonexistence-dockerfile-architect.yml', '-t', '1.0.0', '-a', 'examples'])
+    .command(['register', 'test/mocks/register/nonexistence-dockerfile-architect.yml', '-t', '1.0.0', '-a', 'examples', '-s', 'param_unset=foo'])
     .catch(e => {
       expect(e.message).contains(`${path.resolve('./test/integration/hello-world/nonexistent-dockerfile')} does not exist. Please verify the correct context and/or dockerfile were given.`);
     })