feat: allow for envfile path as a list by dnitsch · Pull Request #54 · Ensono/eirctl

dnitsch · 2025-09-18T16:27:12Z

SUMMARY:

Allow for specification of multiple paths on the envfile property.

Provides recursive merging of envfile properties and path appending for cascading inheritance.

Taking the below example definition eirctl.yaml

contexts:
  withenv:
    envfile:
      path: local/envfile/one.env
      
tasks:
  one: 
    context: withenv
    command: 
      - |
        echo "task one SET_IN_PIPELINE: $SET_IN_PIPELINE"
    envfile:
      path:
        - local/envfile/one.env
  two: 
    command: 
      - | 
        echo "task two SET_IN_PIPELINE: $SET_IN_PIPELINE"
pipelines:
  p1: 
    - task: one
      envfile:
        path:
          - local/envfile/two.env
    - task: two
      env: 
        SET_IN_PIPELINE: bar
      envfile:
        path:
          - local/envfile/one.env
  p2: 
    - pipeline: p1
      envfile:
        path:
          - local/envfile/p2.env

each of the referenced envfiles are merged with the last (or top most specified wins)

if each file contains SET_IN_PIPELINE variable set at a different value -> eirctl run p2

you will get

task two SET_IN_PIPELINE: bar -> as env: {} will directly set will always take highest precedence
task one SET_IN_PIPELINE: p2-override -> the top most specified envfile path will set the value i.e. overwrite any previously set value for the same key in the previous envfiles

if multiple keys are specified that do not clash they will be merged together -> in the specific example for task one, you would end up with this list
envfile.path: ["local/envfile/one.env", "local/envfile/two.env", "local/envfile/p2.env"]
it gets processed in order from left to right

fix: ensure backwards compatibility

docs: update docs around envfile and merging precedence

+semver: FEATURE +semver: feature

dnitsch · 2025-09-19T18:35:53Z

/build-debug

sonarqubecloud · 2025-09-20T11:57:30Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull Request Overview

This PR adds support for specifying multiple envfile paths as a list, providing cascading inheritance and recursive merging of environment variables. The primary change converts envfile.path from a single string to accept either a string or an array of strings.

Updated envfile path handling to support lists with proper merging order
Enhanced test coverage for envfile path precedence and merging behavior
Upgraded Go version and container images to latest versions

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/utils/utils.go	Modified `PathValue` type to support string slice and updated path handling methods
scheduler/stage.go	Added envfile field to Stage struct with getter/setter methods and merging logic
scheduler/denormalize.go	Implemented `envfileMerge` function for proper cascading of envfile paths
schemas/schema_v1.json	Updated JSON schema to support both string and array types for envfile path
variables/variables.go	Updated constructor calls to use `NewVariables()` instead of struct literals
Multiple test files	Enhanced test coverage for envfile path precedence and merging scenarios

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-22T08:48:03Z

 	// since we want to search for a partial match. this loop is required
 	for _, verbotenPair := range forbiddenContainerArgsPairs {
-		slices.ContainsFunc(cargs, func(s string) bool {
+		_ = slices.ContainsFunc(cargs, func(s string) bool {


The result of slices.ContainsFunc is being discarded with _. If the function call has side effects or the result is needed for logic, this should be handled properly.

Copilot · 2025-09-22T08:48:03Z

 	}
-
+
+	// 


Empty comment should be removed or completed with meaningful content.

Suggested change

//

RichardSlater · 2025-09-22T09:01:51Z

    runs-on: ubuntu-24.04
    container:
-      image: golang:1.24.4-bookworm
+      image: public.ecr.aws/docker/library/golang:1-trixie


More of a heads up than a review as I don't think you are using it directly, but pwsh is currently not trivially installable on Debian 13 (Trixie) due to the fact that it's missing libicu =< 74 as it only ships with libicu76:

it's possible to install framework dependant versions of pwsh, but they are 3rd party, not Microsoft; you also need to use .NET 10.

you can also install libicu74 manually using dpkg, by downloading it from the bookworm repo.

my PR that fixes it was merged two weeks ago but hasn't actually made it into the release channel, based upon comments from the Microsoft team it may not do so until .NET 10 is released as they are only targeting libicu76 support from .NET 10 onwards.

This was kind of thought about in a slightly different way and that was to actually create containers with eirctl in it in different flavours one of those would be the sdk8|910 container which just has the binary copied into it. this would give people the ability to leverage an eirct container with a correct shell already.

The only application for this sort of a flow though is when you are defining a runner image e.g. in gitlab or bitbucket where you want to perform some commands in a pwsh before you execute an eirctl pipeline|task.

The other way around you would ideally define a context of microsoft/sdk:8|9|10 which ships with pwsh 7+ and your eirctl task run the commands in pwsh containers already.

Hope that makes sense @RichardSlater but, DM me or reply here, if I missed the point of anything you are trying to say.

That makes sense, as I say it was a heads up that if any PowerShell was needed there is a lack of support from Microsoft for Debian 13.

RichardSlater · 2025-09-22T09:10:11Z

 	}

-	taskRunner := TestTaskRunner{}
+	taskRunner := mockTaskRunner{


There is a lot of repetition here with L103, L149, L181, L225, L269 - could these be refactored into a single common function?

yeah possibly with a t.Helper() signifier - though as they are tests it's OK for them to be a bit wet, there are slight differences between some of them in what they assert and test inside

RichardSlater · 2025-09-22T09:10:47Z

+    - task: golint
+    - task: vuln:check
+    # govet is run as part of golint
+    # - task: govet


New line please :)

RichardSlater · 2025-09-22T09:17:56Z

 - `task` - task to execute on this stage
 - `pipeline` - pipeline to execute on this stage
 - `env` - environment variables. All existing environment variables will be passed automatically
+- `envfile` - envfile specifications allow you to to leverage a cascade of envfile paths and rules.Tthese get merged with  any envfiles specified on the task level.


Suggest defining the order of precedence here too:

an envfile specified with a higher index in the array, i.e. after another, will take precedence over another.

an envfile specified at the task level will take precedence over another specified at the pipeline level.

Additionally 216s/Tthese/ These/g and 216s/ / /g.

RichardSlater · 2025-09-22T09:25:05Z

I was clearly too slow :D

fix: allow for envfile path as array

2de3904

fix: ensure backwards compatibility

dnitsch temporarily deployed to nonprod September 18, 2025 16:27 — with GitHub Actions Inactive

dnitsch changed the title ~~fix: allow for envfile path as array~~ feat: allow for envfile path as a list Sep 19, 2025

feat: add merging

28a818a

docs: update docs around envfile and merging precedence

dnitsch temporarily deployed to nonprod September 19, 2025 08:41 — with GitHub Actions Inactive

fix: update schema

35deb1e

dnitsch temporarily deployed to nonprod September 19, 2025 08:46 — with GitHub Actions Inactive

dnitsch added 2 commits September 19, 2025 14:34

fix: reorg file to follow standards of public/private methods ordering

e14b5b8

feat: correctly merge based on current advertised precedence

2cdbc21

+semver: FEATURE +semver: feature

dnitsch temporarily deployed to nonprod September 19, 2025 17:34 — with GitHub Actions Inactive

fix: tests

ba60bfc

dnitsch temporarily deployed to nonprod September 19, 2025 18:07 — with GitHub Actions Inactive

fix: remove unnecessary parallel

91de5f2

dnitsch temporarily deployed to nonprod September 19, 2025 18:17 — with GitHub Actions Inactive

fix: vulnerability and update linters

ad887c5

dnitsch temporarily deployed to nonprod September 19, 2025 18:23 — with GitHub Actions Inactive

fix: add clean up

e71dd9d

dnitsch temporarily deployed to nonprod September 20, 2025 06:35 — with GitHub Actions Inactive

fix: clarity

c1b16c6

dnitsch temporarily deployed to nonprod September 20, 2025 07:00 — with GitHub Actions Inactive

fix: update schema

8b90251

dnitsch temporarily deployed to nonprod September 20, 2025 11:53 — with GitHub Actions Inactive

RichardSlater requested a review from Copilot September 22, 2025 08:47

Copilot AI reviewed Sep 22, 2025

View reviewed changes

RichardSlater added the enhancement New feature or request label Sep 22, 2025

RichardSlater assigned dnitsch Sep 22, 2025

dnitsch requested review from RichardSlater and russellseymour September 22, 2025 08:56

dnitsch requested a review from ElvenSpellmaker September 22, 2025 08:56

dnitsch merged commit 4614dfb into main Sep 22, 2025
9 checks passed

dnitsch deleted the fix/envfile-pipeline-task branch September 22, 2025 08:58

RichardSlater reviewed Sep 22, 2025

View reviewed changes

Conversation

dnitsch commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnitsch commented Sep 19, 2025

Uh oh!

sonarqubecloud Bot commented Sep 20, 2025

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RichardSlater Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

dnitsch Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

RichardSlater Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

RichardSlater Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

dnitsch Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

RichardSlater Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

RichardSlater Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

RichardSlater commented Sep 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dnitsch commented Sep 18, 2025 •

edited

Loading