Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top-level dependency injections/overrides #808

Open
rmt opened this issue Nov 2, 2023 · 6 comments
Open

Top-level dependency injections/overrides #808

rmt opened this issue Nov 2, 2023 · 6 comments
Labels

Comments

@rmt
Copy link

rmt commented Nov 2, 2023

OpenTofu Version

v1.6.0-dev
on darwin_arm64

Use Cases

When working with third party modules, their dependencies are not always suitable to your use-case, often resulting in confusion, excessive time spent on debugging and trying different approaches, and quite often hacky solutions.

Attempted Solutions

  • passing unnecessarily-derived values as variables to modules to try to establish a dependency (hacky and not always possible)
  • module dependencies (can introduce unintended side effects that are hard to debug)
  • increasing parallelism and running "terraform apply" multiple times (just say no)
  • forking upstream modules, as well as patching modules after init & before apply (unnecessary maintenance burden)
  • submitting patches to upstream modules (great mid/long-term solution, but you still typically have to fork/patch first)
  • multi-stage terraform runs with -target (a workable approach, but not a great user experience)

Proposal

OpenTofu could support a direct way to inject explicit dependencies from the top-level module, or with an extra configuration file specified on the command line.

Ideally, this would use the resource paths returned by "tofu state list", allowing for surgical precision. It could also support wildcards.

Such a feature would put the end-users in control and would be of clear benefit to OpenTofu users, helping to drive its adoption, while maintaining backwards compatability with terraform.

Possible HCL example (only valid in top-level module):

overrides {
    module.eks.resource.x = {
      depends_on = [ module.y.resource.y, resource.z ]  # completely replace dependencies
      depends_on_extra = [ resource.z ]  # add new dependencies
    }
}

Alternative approaches could be:

  • Extend the current overrides feature to cover sub-module-overrides, eg. resource "module.X.type" "name" { depends_on = [] } (sounds nice, but perhaps tricky)
  • A new "before" meta-variable to compliment depends_on, allowing for reverse dependencies for a similar effect [could lead to more incompatible modules with tf, harming adoption]
  • Soft order/dependencies to enable multi-step -target based approach in a single apply run (eg. module X should try to run before module Y, but hard-dependencies still override the order) [depending on how how this is done, semi-deterministic ordering may lead to incompatible modules with tf, and generally lead to modules with poorly tested dependencies]

References

No response

@rmt rmt added enhancement New feature or request pending-decision This issue has not been accepted for implementation nor rejected. It's still open to discussion. labels Nov 2, 2023
@janosdebugs
Copy link
Contributor

Hey @rmt thank you very much for this issue and sorry for the long delay. We have discussed this issue in the core team and feel that overriding the module from the outside is a wrong and potentially dangerous way of going about modifying a module.

As all currently available public modules in the registry are on GitHub, we recommend forking the module in question and maintaining a fork of said module with your desired changes applied. You can submit your fork to the OpenTofu registry here.

Since this feature would require significant work and be potentially dangerous to users, we have decided to not implement this feature at this time and will be closing the issue.

@janosdebugs janosdebugs closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2024
@rmt
Copy link
Author

rmt commented Jan 31, 2024

That's a shame, as such a pragmatic feature would have had a quantifiable benefit, especially for enterprises, whose engineers are often hamstrung by corporate OSS-related processes thanks to legal and liability risks (it's a minefield, and only getting worse, cough EU's CRA cough).

I don't quite understand the perceived danger, though. Outside of the risk that giving someone a hammer is dangerous because they might use it to hammer their own thumb, but giving hammers to users is kind of what TF does.

@Yantrio
Copy link
Member

Yantrio commented Jan 31, 2024

Hi!

When we discussed this in the core team, part of our main concern was that we don't really want to be handing out too many hammers where possible!

I'm aware this could be useful, but it could also be a nightmare for module authors who put a lot of work to ensure things happen in the correct way.

The reason we are recommending forking is because that involves the end user of the module to take the time to understand what is happening inside the module. Rather than just attempting overrides without full knowledge.

This is more about trying to provide a "best practice" and approach to this, and whilst we are aware that forking in the enterprise world can be difficult, we still feel this would be the best approach.

Our opinion here does not mean that we are 100% against this though, If other people feel strongly about this then we are happy to re-evaluate as time goes on.

@janosdebugs
Copy link
Contributor

I'd like to add to the comment by @Yantrio that if we gave users a way to modify the functionality of a module, it's questionable how that would be evaluated under the CRA and you'd likely need to have a chat with legal about this anyway.

@nitrocode
Copy link

nitrocode commented Feb 28, 2024

It's quite possible that upstream modules may use functions that exist in opentofu or terraform, which would exclude the other from using said module.

This opens up another use-case for a way to override or patch upstream modules after initializing (similar to kustomize for helm) modules.

Would opentofu consider reopening this?

Related issue

Now that opentofu is merging new functions

@janosdebugs
Copy link
Contributor

janosdebugs commented Feb 28, 2024

Hi @nitrocode we had a very thorough discussion about the benefits and drawbacks of this feature. It is our unanimous opinion that this is very similar to Aspect Oriented Programming and will lead to fragile code where you have to inspect and patch every single update of a module. This is effectively the same work you would have to do if you were maintaining a fork, except much less transparent. This feature should also not be used to work around legal and compliance requirements.

To ease the pain of working with both Terraform and OpenTofu, the TSC is currently considering #1275 and we were playing with the idea of publishing an OpenTofu polyfill provider for Terraform, but there's no issue for that yet and we haven't given this any serious thought beyond the idea. Equally, if there is functionality missing from OpenTofu that's in Terraform, please open an issue so we can consider it.

Our preferred solution for this would be to:

  1. Convince module authors to use override files once available. (So far we are getting positive feedback from the TSC, no final decision yet.)
  2. Maintain a lightweight fork of the module. (Patching a module with this feature way is effectively the same as maintaining a lightweight fork.)
  3. Convince the module author to use the polyfill for the OpenTofu-specific functions if available to provide Terraform capabilities. (Terraform compatibility wouldn't be solved by the present issue anyway, but this would help module authors.)

I'm happy to reopen the issue for the purposes of collecting a use case where neither of the 3 solutions above are workable and the use case isn't trying to work around legal requirements. (We don't make the laws, we may not agree with them, but I think I speak for all core devs when I say we don't want to provide tools specifically to skirt them.)

@janosdebugs janosdebugs added needs-community-input and removed pending-decision This issue has not been accepted for implementation nor rejected. It's still open to discussion. labels Feb 28, 2024
@janosdebugs janosdebugs reopened this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants