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

INITIAL DRAFT FOR FEEDBACK: Implement provider functions #9990

Closed
wants to merge 11 commits into from

Conversation

SarahFrench
Copy link
Collaborator

@SarahFrench SarahFrench commented Feb 15, 2024

Description

This PR is an initial draft of implementing provider-defined functions for retrieving elements of a resource's self link or id.

Please see internal, shared doc for details about this work.

See comments for version of Terraform required to test locally!

My intention is to split this PR up in future:

I've put release notes in place on this PR that would be split between those future PRs as appropriate!


Release Note Template for Downstream PRs (will be copied)

go get github.com/hashicorp/[email protected]
go get github.com/hashicorp/[email protected]
go get github.com/hashicorp/terraform-plugin-sdk/[email protected]
go mod tidy
…_from_self_link`, acceptance tests and unit tests for shared logic
@SarahFrench
Copy link
Collaborator Author

SarahFrench commented Feb 15, 2024

NOTE: Using Terraform 1.8.0 is required for this feature. I've built 1.8.0 locally from the hashicorp/terraform repo and used that during local development of this PR. I expect the tests above to fail!

I checked out the v1.8.0-alpha20240214 tag and build the provider locally:

% terraform version
Terraform v1.8.0-dev
on darwin_arm64

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 9 files changed, 642 insertions(+), 142 deletions(-))
Terraform Beta: Diff ( 9 files changed, 642 insertions(+), 142 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3445
Passed tests: 3090
Skipped tests: 353
Affected tests: 2

Click here to see the affected service packages
all service packages are affected

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccContainerCluster_withGatewayApiConfig|TestAccWorkstationsWorkstationConfig_updateHostDetails

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withGatewayApiConfig[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 13 files changed, 1046 insertions(+), 142 deletions(-))
Terraform Beta: Diff ( 13 files changed, 1046 insertions(+), 142 deletions(-))

Originally was `resource_name_from_self_link`
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 13 files changed, 1046 insertions(+), 142 deletions(-))
Terraform Beta: Diff ( 13 files changed, 1046 insertions(+), 142 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3451
Passed tests: 3094
Skipped tests: 355
Affected tests: 2

Click here to see the affected service packages
all service packages are affected

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccContainerCluster_withGatewayApiConfig|TestAccWorkstationsWorkstationConfig_updateHostDetails

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withGatewayApiConfig[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

Was previously `project_id_from_id`. This could be misleading if an input string contains a project number.
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 15 files changed, 1202 insertions(+), 142 deletions(-))
Terraform Beta: Diff ( 15 files changed, 1202 insertions(+), 142 deletions(-))

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 15 files changed, 1197 insertions(+), 142 deletions(-))
Terraform Beta: Diff ( 15 files changed, 1197 insertions(+), 142 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3453
Passed tests: 3095
Skipped tests: 351
Affected tests: 7

Click here to see the affected service packages
all service packages are affected

Action taken

Found 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccContainerCluster_withGatewayApiConfig|TestAccProviderFunction_location_from_id|TestAccProviderFunction_name_from_id|TestAccProviderFunction_project_from_id|TestAccProviderFunction_region_from_id|TestAccProviderFunction_zone_from_id|TestAccWorkstationsWorkstationConfig_updateHostDetails

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withGatewayApiConfig[Error message] [Debug log]
TestAccProviderFunction_location_from_id[Error message] [Debug log]
TestAccProviderFunction_name_from_id[Error message] [Debug log]
TestAccProviderFunction_project_from_id[Error message] [Debug log]
TestAccProviderFunction_region_from_id[Error message] [Debug log]
TestAccProviderFunction_zone_from_id[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_updateHostDetails[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took an initial look yesterday and looked reasonable so far. Waiting to review until this moves out of draft.

@SarahFrench
Copy link
Collaborator Author

SarahFrench commented Feb 20, 2024

@rileykarson I marked this PR as out of draft, but I think I'll return it to draft and continue to split it out into multiple PRs now that I've received initial feedback on my approach. The PR is too big for thorough review, but if you have any more general feedback on the approach please post here!

I've broken out these two PRs that could be merged before the functions are:

I'll continue to make PRs for the individual functions and include documentation in each.

@SarahFrench SarahFrench marked this pull request as draft February 20, 2024 17:00
@SarahFrench SarahFrench changed the title INITIAL DRAFT: Implement provider functions INITIAL DRAFT FOR FEEDBACK: Implement provider functions Feb 20, 2024
@SarahFrench
Copy link
Collaborator Author

Now I've received initial feedback, I'm closing this PR in favour of using the FEATURE-BRANCH-provider-functions feature branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants