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

Set 'project' attribute when importing 'google_logging_project_sink' #1019

Merged
merged 3 commits into from
Jan 30, 2018
Merged

Set 'project' attribute when importing 'google_logging_project_sink' #1019

merged 3 commits into from
Jan 30, 2018

Conversation

ewbankkit
Copy link
Contributor

Fixes #1018.

Incorporated the import acceptance test into the main acceptance tests as per current best practice.

Modified acceptance test before setting project attribute:

make testacc TEST=./google TESTARGS='-run=TestAccLoggingProjectSink_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccLoggingProjectSink_basic -timeout 120m
=== RUN   TestAccLoggingProjectSink_basic
--- FAIL: TestAccLoggingProjectSink_basic (5.94s)
	testing.go:513: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
		
		(map[string]string) {
		}
		
		
		(map[string]string) (len=1) {
		 (string) (len=7) "project": (string) (len=11) "root-999999"
		}

and after setting project attribute:

make testacc TEST=./google TESTARGS='-run=TestAccLoggingProjectSink_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccLoggingProjectSink_basic -timeout 120m
=== RUN   TestAccLoggingProjectSink_basic
--- PASS: TestAccLoggingProjectSink_basic (5.56s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	11.741s

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Jan 28, 2018

Computed: true is now set for the project attribute - I think this breaks backwards compatibility. Shall I write a state migration function?
I think we should be OK with setting Computed: true on the project attribute. I was worried about the case where someone's code doesn't specify the attribute and we now store it in state but this is exactly what Computed is for.


func resourceLoggingProjectSinkImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
parts := strings.Split(d.Id(), "/")
if len(parts) != 4 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a regex check and a default project check here? Similar to KMS key ring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Called parseLoggingSinkId.

@@ -1,31 +0,0 @@
package google
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's slightly redundant but I'd prefer not removing this test file and just adding the import tests here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@emilymye
Copy link
Contributor

Thanks!

make testacc TEST=./google TESTARGS='-run=TestAccLoggingProjectSink_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccLoggingProjectSink_ -timeout 120m
=== RUN TestAccLoggingProjectSink_importBasic
=== RUN TestAccLoggingProjectSink_basic
=== RUN TestAccLoggingProjectSink_uniqueWriter
=== RUN TestAccLoggingProjectSink_updatePreservesUniqueWriter
--- PASS: TestAccLoggingProjectSink_importBasic (2.95s)
--- PASS: TestAccLoggingProjectSink_basic (3.21s)
--- PASS: TestAccLoggingProjectSink_uniqueWriter (4.03s)
--- PASS: TestAccLoggingProjectSink_updatePreservesUniqueWriter (6.52s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google6.700s

@emilymye emilymye merged commit f73db84 into hashicorp:master Jan 30, 2018
@ewbankkit ewbankkit deleted the issue-1018 branch January 31, 2018 20:33
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Importing google_logging_project_sink resource fails to set 'project' attribute
2 participants