Merge pull request #170 from github/elbrenn/secret-complete

Complete secrets when calling reusable workflows

languageservice/src/complete.reusable-workflows.test.ts

@@ -5,7 +5,7 @@ import {testFileProvider} from "./test-utils/test-file-provider";
 
 function mapResult(result: CompletionItem[]) {
   return result.map(x => {
-    return {label: x.label, description: (x.documentation as MarkupContent).value};
+    return {label: x.label, description: (x.documentation as MarkupContent)?.value};
   });
 }
 
@@ -24,6 +24,9 @@ jobs:
 
     expect(result).not.toBeUndefined();
     expect(mapResult(result)).toEqual([
+      {
+        label: "key"
+      },
       {
         label: "name",
         description: "An optional name"
@@ -50,10 +53,77 @@ jobs:
 
     expect(result).not.toBeUndefined();
     expect(mapResult(result)).toEqual([
+      {
+        label: "key"
+      },
       {
         label: "name",
         description: "An optional name"
       }
     ]);
   });
+
+  it("completes job secrets", async () => {
+    const input = `
+on: push
+
+jobs:
+  build:
+    uses: ./reusable-workflow-with-inputs.yaml
+    secrets:
+      |
+`;
+    const result = await complete(...getPositionFromCursor(input), {fileProvider: testFileProvider});
+
+    expect(result).not.toBeUndefined();
+    // includes inherit since no secrets have been passed in
+    expect(mapResult(result)).toEqual([
+      {
+        label: "envPAT",
+        description: "A secret for the environment"
+      },
+      {
+        label: "inherit"
+      },
+      {
+        label: "serverPAT"
+      }
+    ]);
+  });
+
+  it("completes inherit secrets", async () => {
+    const input = `
+on: push
+
+jobs:
+  build:
+    uses: ./reusable-workflow-with-inputs.yaml
+    secrets: |
+`;
+    const result = await complete(...getPositionFromCursor(input), {fileProvider: testFileProvider});
+
+    expect(result).not.toBeUndefined();
+    expect(mapResult(result)).toContainEqual({label: "inherit"});
+  });
+
+  it("filters existing secrets", async () => {
+    const input = `
+on: push
+
+jobs:
+  build:
+    uses: ./reusable-workflow-with-inputs.yaml
+    secrets:
+      envPAT: "myPAT"
+      |
+`;
+    const result = await complete(...getPositionFromCursor(input), {fileProvider: testFileProvider});
+
+    expect(result).not.toBeUndefined();
+    expect(mapResult(result)).toEqual([
+      {
+        label: "serverPAT"
+      }
+    ]);
+  });
 });

languageservice/src/complete.ts

@@ -27,7 +27,7 @@ import {findToken} from "./utils/find-token";
 import {guessIndentation} from "./utils/indentation-guesser";
 import {mapRange} from "./utils/range";
 import {getRelCharOffset} from "./utils/rel-char-pos";
-import {transform} from "./utils/transform";
+import {isPlaceholder, transform} from "./utils/transform";
 import {Value, ValueProviderConfig} from "./value-providers/config";
 import {defaultValueProviders} from "./value-providers/default";
 import {definitionValues} from "./value-providers/definition";
@@ -160,7 +160,7 @@ async function getValues(
   const customValueProvider =
     valueProviderToken?.definition?.key && valueProviderConfig?.[valueProviderToken.definition.key];
   if (customValueProvider) {
-    const customValues = await customValueProvider.get(workflowContext);
+    const customValues = await customValueProvider.get(workflowContext, existingValues);
     if (customValues) {
       return filterAndSortCompletionOptions(customValues, existingValues);
     }
@@ -169,7 +169,7 @@ async function getValues(
   const defaultValueProvider =
     valueProviderToken?.definition?.key && defaultValueProviders[valueProviderToken.definition.key];
   if (defaultValueProvider) {
-    const values = await defaultValueProvider.get(workflowContext);
+    const values = await defaultValueProvider.get(workflowContext, existingValues);
     return filterAndSortCompletionOptions(values, existingValues);
   }
 
@@ -209,8 +209,8 @@ export function getExistingValues(token: TemplateToken | null, parent: TemplateT
     const mapKeys = new Set<string>();
     const mapToken = parent as MappingToken;
 
-    for (const {key} of mapToken) {
-      if (isString(key)) {
+    for (const {key, value} of mapToken) {
+      if (isString(key) && !isPlaceholder(key, value)) {
         mapKeys.add(key.value);
       }
     }

languageservice/src/test-utils/test-file-provider.ts

@@ -59,6 +59,13 @@ on:
         description: 'An optional name'
         required: false
         type: string
+      key:
+        type: string
+    secrets:
+      envPAT:
+        required: true
+        description: 'A secret for the environment'
+      serverPAT:
 
 jobs:
   build:

languageservice/src/utils/transform.ts

@@ -1,3 +1,6 @@
+import {isString} from "@github/actions-workflow-parser";
+import {StringToken} from "@github/actions-workflow-parser/templates/tokens/string-token";
+import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token";
 import {Position, TextDocument} from "vscode-languageserver-textdocument";
 import {Range} from "vscode-languageserver-types";
 
@@ -72,3 +75,12 @@ export function transform(doc: TextDocument, pos: Position): [TextDocument, Posi
 
   return [newDoc, newDoc.positionAt(offset)];
 }
+
+// Detect placeholder key and value added by transform
+export function isPlaceholder(key: StringToken, value: TemplateToken) {
+  if (key.value === PLACEHOLDER_KEY && isString(value) && value.value == "") {
+    return true;
+  }
+
+  return false;
+}

languageservice/src/validate.workflow-references.test.ts

@@ -98,6 +98,8 @@ on: push
 jobs:
   build:
     uses: ./reusable-workflow-with-inputs.yaml
+    secrets:
+      envPAT: pat
 `;
     const result = await validate(createDocument("wf.yaml", input), {
       fileProvider: testFileProvider
@@ -129,6 +131,8 @@ jobs:
     uses: ./reusable-workflow-with-inputs.yaml
     with:
       username: monalisa
+    secrets:
+        envPAT: pat
 `;
     const result = await validate(createDocument("wf.yaml", input), {
       fileProvider: testFileProvider

languageservice/src/value-providers/config.ts

@@ -21,7 +21,7 @@ export enum ValueProviderKind {
 
 export type ValueProvider = {
   kind: ValueProviderKind;
-  get: (context: WorkflowContext) => Promise<Value[]>;
+  get: (context: WorkflowContext, existingValues?: Set<string>) => Promise<Value[]>;
 };
 
 export interface ValueProviderConfig {

languageservice/src/value-providers/default.ts

@@ -2,6 +2,7 @@ import {WorkflowContext} from "../context/workflow-context";
 import {ValueProviderConfig, ValueProviderKind} from "./config";
 import {needs} from "./needs";
 import {reusableJobInputs} from "./reusable-job-inputs";
+import {reusableJobSecrets} from "./reusable-job-secrets";
 import {stringsToValues} from "./strings-to-values";
 
 export const DEFAULT_RUNNER_LABELS = [
@@ -28,6 +29,10 @@ export const defaultValueProviders: ValueProviderConfig = {
     kind: ValueProviderKind.AllowedValues,
     get: async context => reusableJobInputs(context)
   },
+  "workflow-job-secrets": {
+    kind: ValueProviderKind.SuggestedValues,
+    get: async (context, existingValues) => reusableJobSecrets(context, existingValues)
+  },
   "runs-on": {
     kind: ValueProviderKind.SuggestedValues,
     get: async (_: WorkflowContext) => stringsToValues(DEFAULT_RUNNER_LABELS)

languageservice/src/value-providers/reusable-job-inputs.ts

@@ -19,7 +19,8 @@ export function reusableJobInputs(context: WorkflowContext): Value[] {
 
     values.push({
       label: key.value,
-      description: inputDescription(value)
+      description: inputDescription(value),
+      insertText: `${key.value}: `
     });
   }
 

languageservice/src/value-providers/reusable-job-secrets.ts

@@ -0,0 +1,53 @@
+import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token";
+import {isMapping, isString} from "@github/actions-workflow-parser/templates/tokens/type-guards";
+import {WorkflowContext} from "../context/workflow-context";
+import {Value} from "./config";
+
+export function reusableJobSecrets(context: WorkflowContext, existingValues?: Set<string>): Value[] {
+  if (!context.reusableWorkflowJob) {
+    return [];
+  }
+
+  const values: Value[] = [];
+
+  const inheritSecrets = context.reusableWorkflowJob["inherit-secrets"];
+  if (inheritSecrets) {
+    return values;
+  }
+
+  // Suggest inherit if no other secrets have been set
+  if (!existingValues || existingValues.size === 0) {
+    values.push({
+      label: "inherit"
+    });
+  }
+
+  if (context.reusableWorkflowJob?.["secret-definitions"]) {
+    for (const {key, value} of context.reusableWorkflowJob["secret-definitions"]) {
+      if (!isString(key)) {
+        continue;
+      }
+
+      values.push({
+        label: key.value,
+        description: secretDescription(value),
+        insertText: `${key.value}: `
+      });
+    }
+  }
+
+  return values;
+}
+
+function secretDescription(secretDef: TemplateToken): string | undefined {
+  if (!isMapping(secretDef)) {
+    return "";
+  }
+
+  const descriptionToken = secretDef.find("description");
+  if (!descriptionToken || !isString(descriptionToken)) {
+    return "";
+  }
+
+  return descriptionToken.value;
+}

workflow-parser/src/model/converter/job.ts

@@ -1,6 +1,6 @@
 import {TemplateContext} from "../../templates/template-context";
 import {BasicExpressionToken, MappingToken, ScalarToken, StringToken, TemplateToken} from "../../templates/tokens";
-import {isSequence, isString} from "../../templates/tokens/type-guards";
+import {isMapping, isSequence, isString} from "../../templates/tokens/type-guards";
 import {Step, WorkflowJob} from "../workflow-template";
 import {convertConcurrency} from "./concurrency";
 import {convertToJobContainer, convertToJobServices} from "./container";
@@ -38,7 +38,9 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
         break;
 
       case "env":
-        env = item.value.assertMapping("job env");
+        handleTemplateTokenErrors(item.value, context, undefined, () => {
+          env = item.value.assertMapping("job env");
+        });
         break;
 
       case "environment":
@@ -69,7 +71,9 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
       }
 
       case "outputs":
-        outputs = item.value.assertMapping("job outputs");
+        handleTemplateTokenErrors(item.value, context, undefined, () => {
+          outputs = item.value.assertMapping("job outputs");
+        });
         break;
 
       case "runs-on":
@@ -95,14 +99,19 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
         break;
 
       case "with":
-        workflowJobInputs = item.value.assertMapping("uses-with value");
+        handleTemplateTokenErrors(item.value, context, undefined, () => {
+          workflowJobInputs = item.value.assertMapping("uses-with value");
+        });
         break;
       case "secrets":
         if (isString(item.value) && item.value.value === "inherit") {
           inheritSecrets = true;
         } else {
-          workflowJobSecrets = item.value.assertMapping("uses-secrets value");
+          handleTemplateTokenErrors(item.value, context, undefined, () => {
+            workflowJobSecrets = item.value.assertMapping("uses-secrets value");
+          });
         }
+        break;
     }
   }