hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.84k stars 9.19k forks source link

Fix `aws_key_pair` import re-creation bug #40075

Open eugercek opened 5 days ago

eugercek commented 5 days ago

Description

Why I Opened this PR: I recently imported many ec2 key pairs in our infra. And it's very toil job. One needs to manually edit state files, which may contains secrets. If we've used many terraform modules in a single root, I probably couldn't edit due to permission restrictions. In some organizations this editing may need many tickets (internal in the company) to resolve.

Why this happens: Currently if one imports a key pair, its public key material is not included. Since there's no a public key (empty) terraform finds a diff between user given value (a proper public_key), even if you write enter correct public_key key, and recreates it.

You can now get public key material of ec2 key pair via DescribeKeyPair api call. It's added in April 28, 2022, relevant doc

||Describe public keys|You can query the public key and creation date of an Amazon EC2 key pair.|April 28, 2022|

For Example (check Example Code for SDK):

aws ec2 describe-key-pairs --key-names key-pair-name --include-public-key

Some catches of this PR

Since we ignored aws_key_pair.public_key in tests we didn't see the problem of aws DescribeKeyPair api. The API overrides the comment you give to the name of the key pair name, if you use console or the sdk. (It's known in data source of key pair, thus its tests handled that.)

Example Code Prints: ``` // ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINNmjoQb6LuFti6eBe/oeTN017N/A22A4ee9H3SJkLty umut-3 // **Not** ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINNmjoQb6LuFti6eBe/oeTN017N/A22A4ee9H3SJkLty umut-custom-comment ``` ```go func main() { cfg, err := config.LoadDefaultConfig(context.TODO()) if err != nil { log.Fatal(err) } svc := ec2.NewFromConfig(cfg) pair, err := svc.ImportKeyPair(context.Background(), &ec2.ImportKeyPairInput{ KeyName: aws.String("umut-3"), PublicKeyMaterial: []byte("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINNmjoQb6LuFti6eBe/oeTN017N/A22A4ee9H3SJkLty umut-custom-comment"), }) if err != nil { log.Fatal(err) } pairs, err := svc.DescribeKeyPairs(context.Background(), &ec2.DescribeKeyPairsInput{ IncludePublicKey: aws.Bool(true), Filters: []types.Filter{ types.Filter{ Name: aws.String("key-pair-id"), Values: []string{*pair.KeyPairId}, }, }, }) if err != nil { log.Fatal() } fmt.Println(*pairs.KeyPairs[0].PublicKey) } ```

That's why I to added DiffSuppressFunc. Currently we put the required argument from operator directly to the state and never compare with real value. Thus that may effect many workflows. With this diff function it won't happen. Changed tests with this in mind.

Relations

Closes #8529 Closes #5347 Closes #1092

Output from Acceptance Testing

> make testacc TESTS='TestAccEC2KeyPair' PKG=ec2 
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.2 test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccEC2KeyPair'  -timeout 360m
2024/11/10 18:24:50 Initializing Terraform AWS Provider...
=== RUN   TestAccEC2KeyPairDataSource_basic
=== PAUSE TestAccEC2KeyPairDataSource_basic
=== RUN   TestAccEC2KeyPairDataSource_includePublicKey
=== PAUSE TestAccEC2KeyPairDataSource_includePublicKey
=== RUN   TestAccEC2KeyPair_basic
=== PAUSE TestAccEC2KeyPair_basic
=== RUN   TestAccEC2KeyPair_tags
=== PAUSE TestAccEC2KeyPair_tags
=== RUN   TestAccEC2KeyPair_nameGenerated
=== PAUSE TestAccEC2KeyPair_nameGenerated
=== RUN   TestAccEC2KeyPair_namePrefix
=== PAUSE TestAccEC2KeyPair_namePrefix
=== RUN   TestAccEC2KeyPair_disappears
=== PAUSE TestAccEC2KeyPair_disappears
=== CONT  TestAccEC2KeyPairDataSource_basic
=== CONT  TestAccEC2KeyPair_nameGenerated
=== CONT  TestAccEC2KeyPair_basic
=== CONT  TestAccEC2KeyPairDataSource_includePublicKey
=== CONT  TestAccEC2KeyPair_tags
=== CONT  TestAccEC2KeyPair_disappears
=== CONT  TestAccEC2KeyPair_namePrefix
--- PASS: TestAccEC2KeyPairDataSource_includePublicKey (28.04s)
--- PASS: TestAccEC2KeyPairDataSource_basic (28.12s)
--- PASS: TestAccEC2KeyPair_disappears (28.23s)
--- PASS: TestAccEC2KeyPair_nameGenerated (31.51s)
--- PASS: TestAccEC2KeyPair_namePrefix (31.70s)
--- PASS: TestAccEC2KeyPair_basic (31.89s)
--- PASS: TestAccEC2KeyPair_tags (59.67s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/ec2        66.441s
github-actions[bot] commented 5 days ago

Community Note

Voting for Prioritization

For Submitters

justinretzolk commented 4 days ago

Related #32502 Related #37713