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.

Sign up for GitHub

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

Jump to bottom

Fix access IDPs not importing config obj #2735

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Fix access IDPs not importing config obj #2735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/2735.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/cloudflare_access_identity_provider: Fix access IDPs not importing config obj
```
16 changes: 4 additions & 12 deletions internal/sdkv2provider/resource_cloudflare_access_identity_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ func resourceCloudflareAccessIdentityProviderRead(ctx context.Context, d *schema
d.Set("name", accessIdentityProvider.Name)
d.Set("type", accessIdentityProvider.Type)

config := convertStructToSchema(d, accessIdentityProvider.Config)
config := convertAccessIDPConfigStructToSchema(accessIdentityProvider.Config)
if configErr := d.Set("config", config); configErr != nil {
return diag.FromErr(fmt.Errorf("error setting Access Identity Provider configuration: %w", configErr))
}

scimConfig := convertScimConfigStructToSchema(d, accessIdentityProvider.ScimConfig)
scimConfig := convertAccessIDPScimConfigStructToSchema(accessIdentityProvider.ScimConfig)
if scimConfigErr := d.Set("scim_config", scimConfig); scimConfigErr != nil {
return diag.FromErr(fmt.Errorf("error setting Access Identity Provider scim configuration: %w", scimConfigErr))
}
Expand Down Expand Up @@ -245,11 +245,7 @@ func convertScimConfigSchemaToStruct(d *schema.ResourceData) cloudflare.AccessId
return ScimConfig
}

func convertStructToSchema(d *schema.ResourceData, options cloudflare.AccessIdentityProviderConfiguration) []interface{} {
if _, ok := d.GetOk("config"); !ok {
return []interface{}{}
}

func convertAccessIDPConfigStructToSchema(options cloudflare.AccessIdentityProviderConfiguration) []interface{} {
attributes := make([]string, 0)
for _, value := range options.Attributes {
attributes = append(attributes, value)
Expand Down Expand Up @@ -285,11 +281,7 @@ func convertStructToSchema(d *schema.ResourceData, options cloudflare.AccessIden
return []interface{}{m}
}

func convertScimConfigStructToSchema(d *schema.ResourceData, options cloudflare.AccessIdentityProviderScimConfiguration) []interface{} {
if _, ok := d.GetOk("scim_config"); !ok {
return []interface{}{}
}

func convertAccessIDPScimConfigStructToSchema(options cloudflare.AccessIdentityProviderScimConfiguration) []interface{} {
m := map[string]interface{}{
"enabled": options.Enabled,
"secret": options.Secret,
Expand Down
50 changes: 49 additions & 1 deletion internal/sdkv2provider/resource_cloudflare_access_identity_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log"
"os"
"strings"
"testing"

"github.com/cloudflare/cloudflare-go"
Expand Down Expand Up @@ -73,6 +74,12 @@ func TestAccCloudflareAccessIdentityProvider_OneTimePin(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, consts.AccountIDSchemaKey, accountID),
resource.TestCheckResourceAttr(resourceName, "name", rnd),
resource.TestCheckResourceAttr(resourceName, "type", "onetimepin"),
resource.TestCheckResourceAttrWith(resourceName, "config.0.redirect_url", func(value string) error {
if !strings.HasSuffix(value, ".cloudflareaccess.com/cdn-cgi/access/callback") {
return fmt.Errorf("expected redirect_url to be a Cloudflare Access URL, got %s", value)
}
return nil
}),
),
},
},
Expand All @@ -90,6 +97,12 @@ func TestAccCloudflareAccessIdentityProvider_OneTimePin(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, consts.ZoneIDSchemaKey, zoneID),
resource.TestCheckResourceAttr(resourceName, "name", rnd),
resource.TestCheckResourceAttr(resourceName, "type", "onetimepin"),
resource.TestCheckResourceAttrWith(resourceName, "config.0.redirect_url", func(value string) error {
if !strings.HasSuffix(value, ".cloudflareaccess.com/cdn-cgi/access/callback") {
return fmt.Errorf("expected redirect_url to be a Cloudflare Access URL, got %s", value)
}
return nil
}),
),
},
},
Expand Down Expand Up @@ -222,6 +235,42 @@ func TestAccCloudflareAccessIdentityProvider_AzureAD(t *testing.T) {
})
}

func TestAccCloudflareAccessIdentityProvider_OAuth_Import(t *testing.T) {
t.Parallel()
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
rnd := generateRandomResourceName()
resourceName := "cloudflare_access_identity_provider." + rnd

checkFn := resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, consts.AccountIDSchemaKey, accountID),
resource.TestCheckResourceAttr(resourceName, "name", rnd),
resource.TestCheckResourceAttr(resourceName, "type", "github"),
resource.TestCheckResourceAttr(resourceName, "config.0.client_id", "test"),
resource.TestCheckResourceAttrSet(resourceName, "config.0.client_secret"),
)

resource.Test(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
testAccPreCheckAccount(t)
},
ProviderFactories: providerFactories,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflareAccessIdentityProviderOAuth(accountID, rnd),
Check: checkFn,
},
{
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceName,
ImportStateIdPrefix: fmt.Sprintf("%s/", accountID),
Check: checkFn,
},
},
})
}

func testAccCheckCloudflareAccessIdentityProviderOneTimePin(name string, identifier *cloudflare.ResourceContainer) string {
return fmt.Sprintf(`
resource "cloudflare_access_identity_provider" "%[1]s" {
Expand Down Expand Up @@ -283,7 +332,6 @@ resource "cloudflare_access_identity_provider" "%[2]s" {
client_id = "test"
client_secret = "test"
directory_id = "directory"
redirect_url = "https://terraform-cfapi.cloudflareaccess.com/cdn-cgi/access/callback"
support_groups = true
conditional_access_enabled = true
}
Expand Down
3 changes: 2 additions & 1 deletion internal/sdkv2provider/schema_cloudflare_access_identity_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func resourceCloudflareAccessIdentityProviderSchema() map[string]*schema.Schema
"config": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The config object might be generated by the API with some default fields.
(Same for scim-config)

Description: "Provider configuration from the [developer documentation](https://developers.cloudflare.com/access/configuring-identity-providers/).",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -136,7 +137,6 @@ func resourceCloudflareAccessIdentityProviderSchema() map[string]*schema.Schema
},
"redirect_url": {
Type: schema.TypeString,
Optional: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Context: redirect_url is always set by the API. Values sent by the client are ignored.

Copy link
Member

Choose a reason for hiding this comment

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

should this be Computed then?

Copy link
Contributor Author

@GreenStage GreenStage Sep 5, 2023 •
edited

Choose a reason for hiding this comment

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

it is 😛 , I just removed the optional

Copy link
Member

Choose a reason for hiding this comment

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

thanks GitHub diffs 🤦‍♂️

Computed: true,
},
"sign_request": {
Expand Down Expand Up @@ -169,6 +169,7 @@ func resourceCloudflareAccessIdentityProviderSchema() map[string]*schema.Schema
"scim_config": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Description: "Configuration for SCIM settings for a given IDP",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down