-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Add support for deprecating variables and (module) outputs #37795
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
base: main
Are you sure you want to change the base?
Conversation
9216b43 to
770948d
Compare
770948d to
82216c0
Compare
cc9d257 to
385d466
Compare
e4f5bea to
b347759
Compare
7a2f5f4 to
e47da40
Compare
e47da40 to
b5016e0
Compare
f2da9cc to
b38e7be
Compare
jbardin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be broken up into 2 or 3 smaller PRs. The new concept of deprecation marks is definitely worthy of a standalone review. Resource attribute deprecation is an existing concept which could then make use of the new mechanism, while module output deprecation is an entirely new addition.
| const TypeType = valueMark("TypeType") | ||
|
|
||
| type DeprecationMark struct { | ||
| Message string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we might want some sort of path info here too, to indicate both the object and attribute path used. The Origin points to the actual source location, but doesn't directly tell the user what was deprecated, and the messages will often not state that explicitly.
Maybe we should add a globalref.Reference, which also includes the source range?
This PR introduces a new attribute called
deprecatedto variable and output blocks. It is a string, that if set indicates that this variable / output should be treated as deprecated.Deprecated variables emit a warning diagnostics if a value is set (be it at the root module level or as part of a module call).
Deprecated outputs emit a warning if they are referenced anywhere. Deprecated outputs can take deprecated values (where e.g. the deprecation comes from another output or from a deprecated resource attribute) without emitting a warning (so that if you are a module author and a module you depend on deprecates an output (or your module has a deprecated resource attribute) you forward you can "forward" the deprecation to the end-consumer of your module (and safely remove it in the next major release of your module). You can not set root module outputs as deprecated, you should be able to remove them safely already.
You will get also deprecation warnings for all illegal uses in modules you consume (as we don't know what is under your area of influence). If a module is outside your control you can use
ignore_nested_deprecationsto ignore any nested deprecation warnings. If the module emits a deprecated value you will still get a warning for that since it's within your control.Fixes #5949
Target Release
1.15.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry