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

Flatten assume_role block for OSS backend #29307

Merged
merged 5 commits into from
Nov 1, 2021
Merged

Conversation

hayorov
Copy link
Contributor

@hayorov hayorov commented Aug 5, 2021

Motivation

Current OSS backend implementation has a nested block in the config section called assume_role that doesn't allow to use it in data source #29305.

This is a suggestion to make the design of the OSS backend (https://www.terraform.io/docs/language/settings/backends/oss.html) more aligned with other existing backends and resolve issues #29305 #23755

Implementation

  • Flatter the assume_role as it implemented in other backends (example s3) as assume_role_X, X nested paremeter
  • Support and deprecate assume_role since 1.1.0 (next release)
  • Add constraints assume_role <> assume_role_X

Params digest with the next flow:

  • populate from deprecated assume_role block
  • (if block empty) populate from flat assuime_role_X

Considerations

This PR introduces backward-compatible changes for the OSS backend and introduces flatter assume_role_* options together with the deprecation of the existing assume_role block with nested parameters.

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 5, 2021

CLA assistant check
All committers have signed the CLA.

@hayorov
Copy link
Contributor Author

hayorov commented Aug 5, 2021

UPD: if the design is acceptable I'm happy to add OSS backend tests and cover assume_role logic, currently there are no tests.

@hayorov hayorov changed the title Flattern assume block parameters for OSS backend Flattern assume_role block for OSS backend Aug 5, 2021
@hayorov
Copy link
Contributor Author

hayorov commented Aug 12, 2021

@jacobm3 up!

@hayorov
Copy link
Contributor Author

hayorov commented Oct 11, 2021

@jbardin it seems that the component maintainer is not available. I've tried all available channels, including Alibaba corporate email and internal Alibaba representatives. What can be next? Any ideas on how to contribute? I'm extremely open to any options.

ping @xiaozhu36
syn @xiaozhu36

@jbardin
Copy link
Member

jbardin commented Oct 11, 2021

Thanks @hayorov, I'm not sure if I can get any more info, but maybe some of our contacts can provide some information. In general, if the code maintainers are not responsive, the backend will be marked as unmaintained, but left available for use for the current major release.

From a quick look at this PR, I doubt it's going to be acceptable since it would be a breaking change to current configuration. They may be amenable to only adding this alternate form, but I cannot speak for the maintainers here.

@xiaozhu36
Copy link
Contributor

HI. @hayorov I am sorry for late reply and I have added my suggestion and please check them.

@hayorov
Copy link
Contributor Author

hayorov commented Oct 12, 2021

Many thanks @xiaozhu36 , let me check and I'll sort them out ...

@hayorov hayorov changed the title Flattern assume_role block for OSS backend Flatten assume_role block for OSS backend Oct 18, 2021
@hayorov
Copy link
Contributor Author

hayorov commented Oct 18, 2021

Dear @xiaozhu36, @jbardin could you please take a look at the update.

Implementation

  • Flatter the assume_role as it implemented in other backends (example s3) as assume_role_X, X nested paremeter
  • Support and deprecate assume_role since 1.1.0 (next release)
  • Add constraints assume_role <> assume_role_X

Params digest with the next flow:

  • populate from deprecated assume_role block
  • (if block empty) populate from flat assuime_role_X

Considerations

This PR introduces backward-compatible changes for the OSS backend and introduces flatter assume_role_* options together with the deprecation of the existing assume_role block with nested parameters.

@hayorov
Copy link
Contributor Author

hayorov commented Oct 21, 2021

@xiaozhu36 comments resolved, please re-review

@hayorov
Copy link
Contributor Author

hayorov commented Oct 22, 2021

Hooray, thx @xiaozhu36

@jbardin please let me know if any additional steps are needed from my side

@hayorov
Copy link
Contributor Author

hayorov commented Oct 27, 2021

Dear, HC maintainers @apparentlymart @jbardin ... others,

it's been a while since the module maintainer has approved my PR and all checks are passed, could you please shed light on the next steps?

@jbardin jbardin self-assigned this Oct 27, 2021
@jbardin
Copy link
Member

jbardin commented Oct 27, 2021

Sorry @hayorov, I didn't see the last ping. I do intend to review this shortly. We have some time before the minor release feature freeze, so there is no worry about this being dropped.

Thanks!

@jbardin
Copy link
Member

jbardin commented Nov 1, 2021

Thanks @hayorov and @xiaozhu36!

@github-actions
Copy link

github-actions bot commented Dec 6, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants