fix: add missing build workflow strategy matrix controls

[giseburt]

Fixes #3499

Feedback extra welcome, as some of this feels extra clumsy

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.36%. Comparing base (02e816d) to head (3c5968a).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3500      +/-   ##
==========================================
+ Coverage   96.34%   96.36%   +0.02%     
==========================================
  Files         192      192              
  Lines       37627    37798     +171     
  Branches     3519     3536      +17     
==========================================
+ Hits        36251    36424     +173     
+ Misses       1376     1374       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[giseburt]

Appears to have worked

[mrgrain]

I'll have a look at the rest later. This feels awfully complicated though. We might need an abstraction here.

[mrgrain]

We need separate PRs for each of these please

[giseburt]

Adjusted back

[giseburt]

I agree completely. It is very hacky at a few points to get this working, partially due to unanticipated needs in the node project object.

I rather liked the interface of this (sister?) project: https://github.com/cdklabs/cdk-pipelines-github/blob/main/src/workflows-model.ts

I don't like that they've put the implementation of the interface in the core of the project: https://github.com/cdklabs/cdk-pipelines-github/blob/main/src/pipeline.ts#L196

But still. I particularly like the handling of inputs and outputs.

Gonna related note, this is another place where Tokens (like in CDK) would come in handy. Was there a historic reason that they weren't put into Projen?

[giseburt]

Ok, so I was able to simplify a bit, and I think it fits the patterns of projen a little better:

const project = new cdk.JsiiProject({
  //...
  buildWorkflowOptions: {
    jobStrategy: {
      matrix: {
        domain: {
          runsOn: ["ubuntu-latest", "windows-latest"],
          node: [
            { version: "18.14.2" },
            { version: "18.18" },
            { version: "18.20" }, // some tools behave differently in 18.20 than 18.18
            { version: "20" },
          ],
        },
        include: [
          {
            runsOn: "ubuntu-latest",
            node: { version: "18.14.2" },
            release: true,
          },
        ],
      },
    },
    nodeVersion: "${{ matrix.node.version }}",
    uploadArtifactsVariable: "matrix.release",
    runsOnVariable: "matrix.runsOn",
  },
  workflowNodeVersion: "18.14.2", // was existing
  // ...

Options to simplify from here:

Assume if buildWorkflowOptions.jobStrategy.matrix.domain.runsOn exists, then:

• use runsOnVariable: "matrix.runsOn" unless overridden

Assume if buildWorkflowOptions.jobStrategy.matrix.domain.node exists, then:

• use nodeVersion: "${{ matrix.node.version }}" unless overridden
• further, if buildWorkflowOptions.jobStrategy.matrix.include does NOT exist, add it as:

  include: [
    {
      runsOn: "ubuntu-latest", // ← if buildWorkflowOptions.jobStrategy.matrix.domain.runsOn
      node: { version: workflowNodeVersion },
      release: true,
    }
  ]

• and uploadArtifactsVariable: "matrix.release" unless overridden

With all that, then this would be the same as above:

const project = new cdk.JsiiProject({
  //...
  buildWorkflowOptions: {
    jobStrategy: {
      matrix: {
        domain: {
          runsOn: ["ubuntu-latest", "windows-latest"],
          node: [
            { version: "18.14.2" },
            { version: "18.18" },
            { version: "18.20" }, // some tools behave differently in 18.20 than 18.18
            { version: "20" },
          ],
        },
      },
    },
  },
  workflowNodeVersion: "18.14.2", // was existing
  // ...

That's a lot of convention over configuration, but that's kinda The Projen Way™.

We could also just say "useBuildStrategyMatrix: true" to default all the above (minus the windows but until we're ready) but I feel that's too presumptive unless we also allow it to be fully overridden.