dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
670 stars 346 forks source link

Can't override jobs.yml parameters like disableComponentGovernance from job #8440

Open akoeplinger opened 2 years ago

akoeplinger commented 2 years ago

Given a .yml like this:

stages:
- stage: build
  jobs:
  - template: /eng/common/templates/jobs/jobs.yml
    parameters:
      jobs:
      - job: Build_Linux
        timeoutInMinutes: 120
        disableComponentGovernance: true    # <--- this causes an error
        steps:
        ....

results in an Azure Pipelines error:

/eng/common/templates/jobs/jobs.yml (Line: 45, Col: 3): 'disableComponentGovernance' is already defined

If I read it correctly this happens because this code passes along parameters from jobs.yml into the job.yml so we end up with one disableComponentGovernance from jobs.yml and one from the job: https://github.com/dotnet/arcade/blob/3cbeb243091dab28adfdcd09fc43b77112cfbe72/eng/common/templates/jobs/jobs.yml#L44-L51

The same happens with the other parameters that are forwarded, like continueOnError etc.

/cc @mmitche

riarenas commented 2 years ago

@akoeplinger Is this blocking things? I think for the particular case of disableComponentGovernance we might just want to get rid of it from jobs.yml, as I don't think we ever want to be in a world where CG is disabled for an entire set of jobs.

mmitche commented 2 years ago

Yeah, that makes sense. I think removing it from the top level is the best thing to do. What jobs do you need to disable CG for?

riarenas commented 2 years ago

What jobs do you need to disable CG for?

I think this is related to the task not working in alpine containers, causing builds to be yellow: https://dev.azure.com/dnceng/internal/_build/results?buildId=1607090&view=results

akoeplinger commented 2 years ago

yes exactly. it's not blocking right now since the task no longer causes an error but the warning is just noise.

Removing the property from jobs.yml sounds like a good solution.

riarenas commented 2 years ago

I'm going to split this issue: