Skip to content

add resource 'api_token' and data source 'permission_groups' #862

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

Merged
merged 20 commits into from
Nov 23, 2020

Conversation

UrosSimovic
Copy link

No description provided.

Unverified

This user has not yet uploaded their public signing key.
@jacobbednarz
Copy link
Contributor

marking as draft as this isn't yet ready for review

@reifnir
Copy link

reifnir commented Nov 10, 2020

marking as draft as this isn't yet ready for review

What is required in order for this to be ready for review?

@jacobbednarz
Copy link
Contributor

jacobbednarz commented Nov 10, 2020

Let's start with a pull request description as to what the change is and any context associated with it. From there we need to:

  • cleanup the comments where functionality isn't used but has been left behind
  • remove the dependency changes to go.mod/go.sum as they come in automatically
  • add documentation for these two new pieces of functionality
  • "WIP" removed from the title

Once we get those things underway, we can start looking at the code itself. Doing these things before someone spends time reviewing the code and approach cuts down on the unnecessary back and forth or scribbling the metaphorical red ink all of the PR.

@jacobbednarz
Copy link
Contributor

Once you remove your changes to go.{mod,sum} you can merge in the latest master and it will include those changes from cloudflare-go.

uros.simovic added 6 commits November 15, 2020 09:39

Unverified

This user has not yet uploaded their public signing key.
# Conflicts:
#	go.mod
#	go.sum

Unverified

This user has not yet uploaded their public signing key.
@UrosSimovic UrosSimovic changed the title WIP: add reasource 'api_token' and data source 'permission_groups' add reasource 'api_token' and data source 'permission_groups' Nov 19, 2020
@UrosSimovic
Copy link
Author

UrosSimovic commented Nov 19, 2020

I think this PR can be reviewed (and docs linted) now:

  • cleanup the comments where functionality isn't used but has been left behind
  • remove the dependency changes to go.mod/go.sum as they come in automatically
  • add documentation for these two new pieces of functionality
  • "WIP" removed from the title

@jacobbednarz jacobbednarz marked this pull request as ready for review November 20, 2020 00:30
@UrosSimovic
Copy link
Author

I hope I've got all proposed changes done.

@jacobbednarz
Copy link
Contributor

Nearly there! It looks like the test state isn't matching up with the assertions right now.

=== RUN   TestAccAPIToken
    testing.go:705: Step 1 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: cloudflare_api_token.uikfrstsfs
          condition.#:                                                                         "1" => "0"
          condition.0.request_ip.#:                                                            "1" => ""
          condition.0.request_ip.0.in.#:                                                       "0" => ""
          condition.0.request_ip.0.not_in.#:                                                   "0" => ""
          id:                                                                                  "b20c3f79a90b9bc33a0cb001ce0606ac" => "b20c3f79a90b9bc33a0cb001ce0606ac"
          name:                                                                                "uikfrstsfs" => "uikfrstsfs"
          policy.#:                                                                            "1" => "1"
          policy.0.effect:                                                                     "deny" => "deny"
          policy.0.permission_groups.#:                                                        "1" => "1"
          policy.0.permission_groups.0:                                                        "82e64a83756745bbbb1c9c2701bf816b" => "82e64a83756745bbbb1c9c2701bf816b"
          policy.0.resources.com.cloudflare.api.account.zone.0da42c8d2132a9ddaf714f9e7c920711: "*" => "*"
          status:                                                                              "active" => "active"
          value:                                                                               "mgdSw-BoxExx7F2Q1me4SAqXDGpfoc07NDDBCV74" => "mgdSw-BoxExx7F2Q1me4SAqXDGpfoc07NDDBCV74"



        STATE:

        cloudflare_api_token.uikfrstsfs:
          ID = b20c3f79a90b9bc33a0cb001ce0606ac
          provider = provider.cloudflare
          condition.# = 1
          condition.0.request_ip.# = 1
          name = uikfrstsfs
          policy.# = 1
          policy.0.effect = deny
          policy.0.permission_groups.# = 1
          policy.0.permission_groups.0 = 82e64a83756745bbbb1c9c2701bf816b
          policy.0.resources.com.cloudflare.api.account.zone.0da42c8d2132a9ddaf714f9e7c920711 = *
          status = active
          value = mgdSw-BoxExx7F2Q1me4SAqXDGpfoc07NDDBCV74
--- FAIL: TestAccAPIToken (9.48s)
=== RUN   TestAccAPITokenAllowDeny
    testing.go:705: Step 0 error: After applying this step and refreshing, the plan was not empty:

        DIFF:

        UPDATE: cloudflare_api_token.ywpzjtdhmx
          condition.#:                                                                         "1" => "0"
          condition.0.request_ip.#:                                                            "1" => ""
          condition.0.request_ip.0.in.#:                                                       "0" => ""
          condition.0.request_ip.0.not_in.#:                                                   "0" => ""
          id:                                                                                  "6aa4d5d28e95f8fd56f8bd27ccdb70e1" => "6aa4d5d28e95f8fd56f8bd27ccdb70e1"
          name:                                                                                "ywpzjtdhmx" => "ywpzjtdhmx"
          policy.#:                                                                            "1" => "1"
          policy.0.effect:                                                                     "allow" => "allow"
          policy.0.permission_groups.#:                                                        "1" => "1"
          policy.0.permission_groups.0:                                                        "82e64a83756745bbbb1c9c2701bf816b" => "82e64a83756745bbbb1c9c2701bf816b"
          policy.0.resources.com.cloudflare.api.account.zone.0da42c8d2132a9ddaf714f9e7c920711: "*" => "*"
          status:                                                                              "active" => "active"
          value:                                                                               "bysG-69wNNXXOagUicOqRlXONjKKmNGYjP9dGDvB" => "bysG-69wNNXXOagUicOqRlXONjKKmNGYjP9dGDvB"



        STATE:

        cloudflare_api_token.ywpzjtdhmx:
          ID = 6aa4d5d28e95f8fd56f8bd27ccdb70e1
          provider = provider.cloudflare
          condition.# = 1
          condition.0.request_ip.# = 1
          name = ywpzjtdhmx
          policy.# = 1
          policy.0.effect = allow
          policy.0.permission_groups.# = 1
          policy.0.permission_groups.0 = 82e64a83756745bbbb1c9c2701bf816b
          policy.0.resources.com.cloudflare.api.account.zone.0da42c8d2132a9ddaf714f9e7c920711 = *
          status = active
          value = bysG-69wNNXXOagUicOqRlXONjKKmNGYjP9dGDvB
--- FAIL: TestAccAPITokenAllowDeny (4.17s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	13.690s
FAIL

@UrosSimovic
Copy link
Author

Had to make some if checks. And to change policy blocks from list to hash set, as CF is returning policies unordered.

@jacobbednarz
Copy link
Contributor

nice one! looks like we're all green on the integration test suite.

=== RUN   TestAccAPIToken
--- PASS: TestAccAPIToken (12.25s)
=== RUN   TestAccAPITokenAllowDeny
--- PASS: TestAccAPITokenAllowDeny (20.38s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	32.661s

@jacobbednarz
Copy link
Contributor

Datasource is also green now after fixing the ID

=== RUN   TestAccCloudflareApiTokenPermissionGroups
--- PASS: TestAccCloudflareApiTokenPermissionGroups (2.41s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	2.433s

@jacobbednarz jacobbednarz changed the title add reasource 'api_token' and data source 'permission_groups' add resource 'api_token' and data source 'permission_groups' Nov 23, 2020
@jacobbednarz jacobbednarz merged commit 7f1466b into cloudflare:master Nov 23, 2020
@jacobbednarz
Copy link
Contributor

Thanks for the persistence on this one @UrosSimovic! Appreciate your contribution here 👍

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.

None yet

3 participants