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

Add resource gitlab_user_gpgkey #1181

Merged
merged 1 commit into from Aug 5, 2022
Merged

Add resource gitlab_user_gpgkey #1181

merged 1 commit into from Aug 5, 2022

Conversation

yan12125
Copy link
Contributor

Description

Somehow terraform plan always reports the field armored_public_key as
changed.

Closes https://github.com/gitlabhq/terraform-provider-gitlab/issues/1166

PR Checklist Acknowledgement

  • I acknowledge that all of the following items are true, where applicable:
    • Resource attributes match 1:1 the names and structure of the API resource in the GitLab API documentation.
    • Examples are updated with:
      • A *.tf file for the resource/s with at least one usage example
      • A *.sh file for the resource/s with an import example (if applicable)
    • New resources have at minimum a basic test with three steps:
      • Create the resource
      • Update the attributes
      • Import the resource
    • No new //lintignore comments were copied from existing code. (Linter rules are meant to be enforced on new code.)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @yan12125 👋

It looks like this is your first submission to the Terraform GitLab Provider! If you haven’t already done so, please make sure you have checked out our CONTRIBUTING.md guide to make sure your contribution has all the necessary elements in place for a successful approval.

Thanks again, and welcome to the community! 😃

@yan12125
Copy link
Contributor Author

Somehow terraform plan always reports the field key as changed.

Here are steps to reproduce:

  • Create a resource like this:
resource "gitlab_user_gpgkey" "yan12125" {
    key = <<KEY
-----BEGIN PGP PUBLIC KEY BLOCK-----

xxx
-----END PGP PUBLIC KEY BLOCK-----
KEY
}
  • Import the existing key with terraform import gitlab_user_gpgkey.yan12125 123, where 123 is the key ID for the current user.
  • terraform plan reports changed resource: (personal information removed)
Warning: Provider development overrides are in effect

The following provider development overrides are set in the CLI configuration:
 - gitlabhq/gitlab in /home/yen/Projects/terraform-provider-gitlab

The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
gitlab_user_gpgkey.yan12125: Refreshing state... [id=130860]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # gitlab_user_gpgkey.yan12125 must be replaced
-/+ resource "gitlab_user_gpgkey" "yan12125" {
      ~ created_at = "2022-07-19T16:45:31Z" -> (known after apply)
      ~ id         = "123" -> (known after apply)
      ~ key        = <<-EOT # forces replacement
            -----BEGIN PGP PUBLIC KEY BLOCK-----
            
            xxx
            -----END PGP PUBLIC KEY BLOCK-----
        EOT
      ~ key_id     = 123 -> (known after apply)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

─────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't
guarantee to take exactly these actions if you run "terraform apply" now.

The key recorded in terraform.tfstate is the same as my actual key, but somehow my code makes terraform treat them as different.

@timofurrer
Copy link
Member

@yan12125 The diff is because with your <<KEY .. KEY syntax the key has a trailing newline which is removed by GitLab. It's a similar situation as I've described in https://github.com/gitlabhq/terraform-provider-gitlab/issues/1174#issuecomment-1184056524.

I suggest that you remove leading and trailing whitespaces from the key in the create function.

@yan12125
Copy link
Contributor Author

Thanks! Now terraform plan no longer reports differences. Next steps include adding tests and docs. I will adding tests first.

@yan12125 yan12125 changed the title [WIP] Add resource gitlab_user_gpgkey Add resource gitlab_user_gpgkey Jul 25, 2022
@yan12125 yan12125 marked this pull request as ready for review July 25, 2022 05:24
@yan12125
Copy link
Contributor Author

Added docs and tests - now ready for reviews :)

@timofurrer timofurrer self-requested a review July 26, 2022 12:12
Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

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

It already looks quite good! I've added a few nitpicks and other comments.

Back to you 🏓

internal/provider/resource_gitlab_user_gpgkey.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_user_gpgkey.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_user_gpgkey.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_user_gpgkey.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_user_gpgkey.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_user_gpgkey_test.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@yan12125
Copy link
Contributor Author

Thanks! I applied some trivial changes. Others need some time :)

@timofurrer timofurrer added this to the v3.17.0 milestone Jul 26, 2022
@github-actions github-actions bot added the merge-conflict PR cannot be merged due to a merge conflict label Aug 2, 2022
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

This pull request has merge conflicts. Please rebase your branch onto main.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

Conflicts are resolved. Thank you! 😀

@github-actions github-actions bot removed the merge-conflict PR cannot be merged due to a merge conflict label Aug 2, 2022
@yan12125
Copy link
Contributor Author

yan12125 commented Aug 2, 2022

Rebased on top of master as upgrading go-gitlab conflicts with upgrades of other dependencies (#1184)

Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

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

A few minor things and make sure to run make reviewable before the next push, to re-generate the docs etc.

Back to you 🏓

examples/resources/gitlab_user_gpgkey/resource.tf Outdated Show resolved Hide resolved
examples/resources/gitlab_user_gpgkey/resource.tf Outdated Show resolved Hide resolved
@timofurrer
Copy link
Member

@yan12125 I'm updating go-gitlab via #1195

@yan12125
Copy link
Contributor Author

yan12125 commented Aug 3, 2022

Thanks for the reminder about make renewable. I run it locally with Go 1.19, and it reports that io/ioutil is deprecated. I didn't find that error on CI checks, so I keep that as-is.

internal/provider/config.go:7:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
        "io/ioutil"
        ^

@timofurrer
Copy link
Member

@yan12125 Can you change that code in the config.go file to use os and os.ReadFile instead of ioutils?

@yan12125
Copy link
Contributor Author

yan12125 commented Aug 4, 2022

Can you change that code in the config.go file to use os and os.ReadFile instead of ioutils?

I can try it out. Is it better to make changes in this PR or another PR?

@timofurrer
Copy link
Member

@yan12125 let's do another PR, since you've asked ;)

@yan12125
Copy link
Contributor Author

yan12125 commented Aug 4, 2022

let's do another PR, since you've asked ;)

Here you go~ #1199


Regarding this PR, I rebased on top of the latest main branch and drop changes to go.mod/go.sum.

@timofurrer
Copy link
Member

@yan12125 I think we are just a rebase away 🎉 (I've merged #1199)

* Using strings.TrimSpace() as GitLab and/or Terraform may add trailing
  newlines to field values.

* Add tests for resource gitlab_user_gpgkey

* Add tests for resource gitlab_user_gpgkey without user_id

* Improve docs and add examples

* Update internal/provider/resource_gitlab_user_gpgkey.go

* Update internal/provider/resource_gitlab_user_gpgkey.go

* Update internal/provider/resource_gitlab_user_gpgkey.go

* Update internal/provider/resource_gitlab_user_gpgkey.go

* Use simpler GetGPGKey & GetGPGKeyForUser APIs

* Include the config in the test as suggested in #1008

* Rely only on ImportStateVerify and get rid of manual checks

* Also use simpler GetGPGKey & GetGPGKeyForUser APIs for tests

* Drop the check for the created_at field in GPG keys

* Also test switching between the current user and a specific user

* Fix the example and regenerate docs

* Fix "ineffectual assignment" errors from golangci-lint

* Run gofmt

Closes https://github.com/gitlabhq/terraform-provider-gitlab/issues/1166

Co-authored-by: Timo Furrer <tuxtimo@gmail.com>
@yan12125
Copy link
Contributor Author

yan12125 commented Aug 5, 2022

Great thanks! Rebased and squashed :)

@yan12125
Copy link
Contributor Author

yan12125 commented Aug 5, 2022

Test failures seem unrelated. Both 15.1.3-ce.0 and 15.1.3-ee.0 tests fail with:

    resource_gitlab_project_test.go:121: Step 19/19 error: After applying this test step and performing a `terraform refresh`, the plan was not empty.
        stdout
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # gitlab_project.foo will be updated in-place
          ~ resource "gitlab_project" "foo" {
                id                                               = "93"
                name                                             = "foo-1604740959322635418"
                tags                                             = [
                    "bar",
                    "foo",
                ]
                # (63 unchanged attributes hidden)
        
              ~ container_expiration_policy {
                  ~ enabled     = false -> true
                    # (4 unchanged attributes hidden)
                }
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.

Which appears the same issue as https://github.com/gitlabhq/terraform-provider-gitlab/issues/1039.

Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@timofurrer
Copy link
Member

@yan12125 I've re-triggered the tests and it's all looking good now. Nice work 🎉 🚢

@timofurrer timofurrer merged commit 394f4ff into gitlabhq:main Aug 5, 2022
@yan12125 yan12125 deleted the add-resource-gitlab_user_gpgkey branch August 5, 2022 09:15
@yan12125
Copy link
Contributor Author

yan12125 commented Aug 5, 2022

Thank you very much for the time!

@github-actions
Copy link

This functionality has been released in v3.17.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants