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

ephemeral: add ephemeral_google_service_account_key #12143

Draft
wants to merge 4 commits into
base: FEATURE-BRANCH-ephemeral-resource
Choose a base branch
from

Conversation

BBBmau
Copy link
Collaborator

@BBBmau BBBmau commented Oct 25, 2024

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.


@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.

google provider: Diff ( 2 files changed, 120 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 2 files changed, 120 insertions(+), 1 deletion(-))

Errors

google provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

google-beta provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

All service packages are affected

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

@BBBmau BBBmau force-pushed the support-service-account-key branch from 6b8f7e5 to 083f850 Compare November 7, 2024 05:03
@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.

google provider: Diff ( 2 files changed, 128 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 2 files changed, 128 insertions(+), 1 deletion(-))

Errors

google provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

google-beta provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

@BBBmau
Copy link
Collaborator Author

BBBmau commented Nov 7, 2024

@SarahFrench when it comes to testing this ephemeral we'll have to do the following:

Get in contact with the google team to setup a key that we can have access for testing purposes (in order for us to consistently be able to test the ephemeral resource with a single service account key)

This is necessary since we are unable to have the ephemeral wait for a rsource to create a service key that we can use (it is called at plan/apply time and would result in getting an empty value)

I'm unsure of another approach. let me know your thoughts on this! I was also thinking about the value assertion which is also another option I can look at in the morning.

@SarahFrench
Copy link
Collaborator

SarahFrench commented Nov 7, 2024

@SarahFrench when it comes to testing this ephemeral we'll have to do the following:

Get in contact with the google team to setup a key that we can have access for testing purposes (in order for us to consistently be able to test the ephemeral resource with a single service account key)

This is necessary since we are unable to have the ephemeral wait for a rsource to create a service key that we can use (it is called at plan/apply time and would result in getting an empty value)

I'm unsure of another approach. let me know your thoughts on this! I was also thinking about the value assertion which is also another option I can look at in the morning.

Does the ephemeral resource not get affected by the resource graph? I.e. like in this test for the equivalent data source the dependency on the google_service_account_key resource means the data source will need to wait for the resource it depends on to exist.

I'd be surprised if ephemeral resource are not affected by the resource graph, but if they aren't we can just split the test into 2 steps:

  1. Provision google_service_account and google_service_account_key managed resources
  2. Update the config to also include ephemeral.google_service_account_key in step 2 of the test

@BBBmau
Copy link
Collaborator Author

BBBmau commented Nov 7, 2024

@SarahFrench when it comes to testing this ephemeral we'll have to do the following:
Get in contact with the google team to setup a key that we can have access for testing purposes (in order for us to consistently be able to test the ephemeral resource with a single service account key)
This is necessary since we are unable to have the ephemeral wait for a rsource to create a service key that we can use (it is called at plan/apply time and would result in getting an empty value)
I'm unsure of another approach. let me know your thoughts on this! I was also thinking about the value assertion which is also another option I can look at in the morning.

Does the ephemeral resource not get affected by the resource graph? I.e. like in this test for the equivalent data source the dependency on the google_service_account_key resource means the data source will need to wait for the resource it depends on to exist.

I'd be surprised if ephemeral resource are not affected by the resource graph, but if they aren't we can just split the test into 2 steps:

1. Provision google_service_account and google_service_account_key managed resources

2. Update the config to also include ephemeral.google_service_account_key in step 2 of the test

I'll separate it into 2 steps then.

@SarahFrench I had attempted to replicate what you linked in terms of creating a google_service_account_key prior to getting with data source (in our case the ephemeral, however I ran into issues when it came to the key not being created prior to the ephemeral. After discussion it looks to be that the ephemeral resource does not get affected by the resource graph. You can look at the discussions here: https://hashicorp.slack.com/archives/C071HC4JJCC/p1730923120107199

@SarahFrench
Copy link
Collaborator

🤯 that's unexpected that they don't use the resource graph! Then definitely a test with 2 steps seems to be the only option for now, until subsequent updates happen.

@BBBmau BBBmau force-pushed the support-service-account-key branch from 083f850 to 994bf3c Compare November 8, 2024 09:14
@BBBmau
Copy link
Collaborator Author

BBBmau commented Nov 8, 2024

Marking ready for review after rebasing and adding tests:

(base) ┌─(~/Dev/terraform-provider-google)───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────(mau@mau-JKDT676NCP:s101)─┐
└─(01:14:02 on main ✹ ✭)──> envchain GCLOUD make test TESTARGS='-run=TestAccEphemeralServiceAccountKey_basic ./google/services/resourcemanager/...'                                                                               ──(Fri,Nov08)─┘
sh -c "'/Users/mau/Dev/terraform-provider-google/scripts/gofmtcheck.sh'"
==> Checking that code complies with gofmt requirements...
go vet
go test -run=TestAccEphemeralServiceAccountKey_basic ./google/services/resourcemanager/... -timeout=30s $(go list -e ./... | grep -v github.com/hashicorp/terraform-provider-google/scripts)
ok      github.com/hashicorp/terraform-provider-google/google/services/resourcemanager  1.481s

@BBBmau BBBmau marked this pull request as ready for review November 8, 2024 09:15
@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.

google provider: Diff ( 3 files changed, 194 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 194 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 4268
Passed tests: 3835
Skipped tests: 417
Affected tests: 16

Click here to see the affected service packages

All service packages are affected

Action taken

Found 16 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccBackupDRBackupVault_fullUpdate
  • TestAccContainerCluster_withEnableKubernetesAlpha
  • TestAccContainerCluster_withEnableKubernetesBetaAPIs
  • TestAccContainerCluster_withEnableKubernetesBetaAPIsOnExistingCluster
  • TestAccContainerCluster_withEnablePrivateEndpointToggle
  • TestAccContainerCluster_withExternalIpsConfig
  • TestAccContainerCluster_withSecretManagerConfig
  • TestAccContainerCluster_withVersion
  • TestAccContainerNodePool_regionalAutoscaling
  • TestAccContainerNodePool_resourceManagerTags
  • TestAccDNSResponsePolicy_dnsResponsePolicyBasicExample
  • TestAccEphemeralServiceAccountKey_basic
  • TestAccGKEBackupBackupPlan_update
  • TestAccGKEBackupRestorePlan_gkebackupRestoreplanRollbackNamespaceExample
  • TestAccIapWebTypeComputeIamBindingGenerated
  • TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityBasicExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccBackupDRBackupVault_fullUpdate [Debug log]
TestAccContainerCluster_withEnableKubernetesAlpha [Debug log]
TestAccContainerCluster_withEnableKubernetesBetaAPIs [Debug log]
TestAccContainerCluster_withEnableKubernetesBetaAPIsOnExistingCluster [Debug log]
TestAccContainerCluster_withEnablePrivateEndpointToggle [Debug log]
TestAccContainerCluster_withExternalIpsConfig [Debug log]
TestAccContainerCluster_withVersion [Debug log]
TestAccContainerNodePool_regionalAutoscaling [Debug log]
TestAccDNSResponsePolicy_dnsResponsePolicyBasicExample [Debug log]
TestAccGKEBackupBackupPlan_update [Debug log]
TestAccGKEBackupRestorePlan_gkebackupRestoreplanRollbackNamespaceExample [Debug log]
TestAccIapWebTypeComputeIamBindingGenerated [Debug log]
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🔴 Tests failed during RECORDING mode:
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]
TestAccContainerNodePool_resourceManagerTags [Error message] [Debug log]
TestAccEphemeralServiceAccountKey_basic [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@SarahFrench
Copy link
Collaborator

Let's focus on the other PRs you have in flight before moving on to reviewing this one

@SarahFrench SarahFrench marked this pull request as draft November 8, 2024 12:41
@BBBmau
Copy link
Collaborator Author

BBBmau commented Nov 13, 2024

after rebasing no other changes were needed in order for this to be ready for review. I'll leave it as draft until the two PRs in review have been merged:

@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.

google provider: Diff ( 3 files changed, 194 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 194 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 4268
Passed tests: 3845
Skipped tests: 417
Affected tests: 6

Click here to see the affected service packages

All service packages are affected

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccBackupDRBackupVault_fullUpdate
  • TestAccCloudRunService_cloudRunServiceMulticontainerExample
  • TestAccCloudbuildv2Connection_GlePrivConnection
  • TestAccCloudbuildv2Connection_GlePrivUpdateConnection
  • TestAccContainerCluster_withSecretManagerConfig
  • TestAccEphemeralServiceAccountKey_basic

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccBackupDRBackupVault_fullUpdate [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🔴 Tests failed during RECORDING mode:
TestAccCloudRunService_cloudRunServiceMulticontainerExample [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivConnection [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivUpdateConnection [Error message] [Debug log]
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]
TestAccEphemeralServiceAccountKey_basic [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

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