diggerhq / digger

Digger is an open source IaC orchestration tool. Digger allows you to run IaC in your existing CI pipeline ⚡️
https://digger.dev
Apache License 2.0
2.84k stars 129 forks source link

'dependency_configuration' is undocumented and possibly has wrong default behaviour #1262

Open ahuseby opened 5 months ago

ahuseby commented 5 months ago

Use of dependency_configuration is evidently supported, but lacks documentation in the Digger user documentation. I'm not sure if https://github.com/diggerhq/digger/issues/782 is supposed to address this.

Furthermore dependency_configuration could potetially have the wrong default behaviour, based on the definition of the two available modes (hard and soft). https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/digger_config/converters.go#L12-L17

My experience is that without specifying anything for dependency_configuration.mode, the following happens

We have 2 projects, A and B B is dependent on A (using 'depends_on')

We want to make a change that only affects B

  1. If we commit a change that only affects B, nothing happens.
  2. If we then add a commit with change affecting A, then a plan run starts for A and B.

The way I observe this, in regards to adjusting dependency_configuration and how it currently functions

However, the definition from the source code tell us the opposite.

// soft - if dependency project wasn't changed, it will be skipped

So seemingly the actual observed default behaviour is that of "soft", by definition.

Is the issue perhaps on this line in the orchestrator code? https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/orchestrator/github/github.go#L476 Perhaps change the condition to diggerConfig.DependencyConfiguration.Mode == digger_config.DependencyConfigurationSoft?

I have discussed this with and demonstrated it on video call to @motatoes. In those discussions I've described a transitive dependency C -> B -> A, but that doesn't seem to matter at all. With the default behaviour, for C to plan, I have to make a change affecting A aswell.

ahuseby commented 5 months ago

I'm happy to contribute changes, but first need to clarify what the definition of DependencyConfiguration.Mode is and what the default behaviour is expected to be 🙂

motatoes commented 5 months ago

I've discussed with @Spartakovic regarding this and he said that there might be a bug indeed with the "hard" behaviour and asked me to try and replace the line https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/orchestrator/github/github.go#L520 to true might be the fix. Going to further investigate

ahuseby commented 5 months ago

Please let me know if you need someone to test this :)