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 support for Access Identity Provider #597

Merged

Conversation

jacobbednarz
Copy link
Member

Introduces full support for Cloudflare Access Identity Providers.

API documentation: https://api.cloudflare.com/#access-identity-providers-properties
Developer documentation: https://developers.cloudflare.com/access/configuring-identity-providers/

@ghost ghost added size/XL kind/documentation Categorizes issue or PR as related to documentation. labels Feb 12, 2020
@jacobbednarz
Copy link
Member Author

Depends on cloudflare/cloudflare-go#403 and build will be red until that lands.

"client_secret": {
Type: schema.TypeString,
Optional: true,
// client_secret is a write only operation from the Cloudflare API
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still not totally sold on this however it's the most elegant way of managing the write-but-never-retrievable situation the client_secret and idp_public_cert fields are in.

IDPConfig.Attributes = attrData
}

IDPConfig.AppsDomain = d.Get("config.0.apps_domain").(string)
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is a little verbose but I don't mind it as it's explicit and it doesn't require confusing reflection to work out what each field should be.

@jacobbednarz jacobbednarz force-pushed the access-identity-provider-support branch from eda5fa8 to 72ca699 Compare February 19, 2020 22:12
@jacobbednarz
Copy link
Member Author

Got a couple of integration error failures now that I'll need to checkout

------- Stdout: -------
=== RUN   TestAccCloudflareAccessIdentityProviderOAuth
=== PAUSE TestAccCloudflareAccessIdentityProviderOAuth
=== CONT  TestAccCloudflareAccessIdentityProviderOAuth
--- FAIL: TestAccCloudflareAccessIdentityProviderOAuth (0.62s)
    testing.go:654: Step 0 error: errors during apply:
        
        Error: error creating Access Identity Provider for ID "": error unmarshalling the JSON response: json: cannot unmarshal object into Go struct field AccessIdentityProviderListResponse.messages of type string
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test391709182/main.tf line 2:
          (source code not available)
        
        
FAIL
------- Stdout: -------
=== RUN   TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
=== PAUSE TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
=== CONT  TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
--- FAIL: TestAccCloudflareAccessIdentityProviderOAuthWithUpdate (0.62s)
    testing.go:654: Step 0 error: errors during apply:
        
        Error: error creating Access Identity Provider for ID "": error unmarshalling the JSON response: json: cannot unmarshal object into Go struct field AccessIdentityProviderListResponse.messages of type string
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test813874740/main.tf line 2:
          (source code not available)
        
        
FAIL

Copy link
Contributor

@patryk patryk left a comment

Choose a reason for hiding this comment

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

Generally all is fine, but we need to fix tests and merge conflict.

@patryk patryk added the wip label Mar 2, 2020
jacobbednarz referenced this pull request in jacobbednarz/cloudflare-go Mar 8, 2020
When the work for
terraform-providers/terraform-provider-cloudflare#597 was started,
the `messages` object was either empty array or a single string (per
the original `cloudflare.Response`[1] struct) but the API response is
now spitting out an array of `message` objects inside of the current
array.

To fix the issue, the two structs have been updated to conform to the
required objects.

[1]: https://github.com/cloudflare/cloudflare-go/blob/master/cloudflare.go#L363
@jacobbednarz
Copy link
Member Author

The issue for the two CI failures above have been fixed in cloudflare/cloudflare-go#425 due to the underlying API response changing. Once that is merged, we're 👌 to merge this.

patryk referenced this pull request in cloudflare/cloudflare-go Mar 9, 2020
* Update `*ListResponse` structs to match API responses

When the work for
terraform-providers/terraform-provider-cloudflare#597 was started,
the `messages` object was either empty array or a single string (per
the original `cloudflare.Response`[1] struct) but the API response is
now spitting out an array of `message` objects inside of the current
array.

To fix the issue, the two structs have been updated to conform to the
required objects.

[1]: https://github.com/cloudflare/cloudflare-go/blob/master/cloudflare.go#L363

* Use Response shared struct for handling fields

Co-authored-by: Patryk Szczygłowski <patryk@patryk.net>
@patryk
Copy link
Contributor

patryk commented Mar 9, 2020

Thanks for investigation. It looks that the API has actually been fixed 🙄

@jacobbednarz jacobbednarz merged commit c4667a4 into cloudflare:master Mar 9, 2020
@jacobbednarz jacobbednarz deleted the access-identity-provider-support branch March 9, 2020 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants