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

DLP-655 Adds support for DLP profiles #1984

Merged
merged 3 commits into from
Oct 31, 2022
Merged

DLP-655 Adds support for DLP profiles #1984

merged 3 commits into from
Oct 31, 2022

Conversation

GreenStage
Copy link
Contributor

@GreenStage GreenStage commented Oct 20, 2022 •
edited
Loading

Builds will fail as this depends on the next cloudflare-go release

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2022 •
edited
Loading

changelog detected ✅

@GreenStage GreenStage force-pushed the egomes/DLP-655 branch 5 times, most recently from 42b532e to dbf7cf6 Compare October 26, 2022 11:55
@GreenStage
Copy link
Contributor Author

GreenStage commented Oct 26, 2022 •
edited
Loading

first merge #1995

or decline it and merge only this one

@GreenStage GreenStage force-pushed the egomes/DLP-655 branch 2 times, most recently from d36130f to 96a6b00 Compare October 26, 2022 14:22
jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
.changelog/1984.txt Outdated
Comment on lines 1 to 3
```release-note:enhancement
resource/cloudflare_dlp_profile: adds support for DLP profiles
```
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
```release-note:enhancement
resource/cloudflare_dlp_profile: adds support for DLP profiles
```
```release-note:new-resource
cloudflare_dlp_profile
```

jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
internal/provider/resource_cloudflare_dlp_profile.go Outdated

identifier := cloudflare.AccountIdentifier(d.Get("account_id").(string))
dlpProfile, err := client.GetDLPProfile(ctx, identifier, d.Id())
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

you'll want to be more specific here as any error that bubbles up from GetDLPProfile will remove the DLP profile from state.

Suggested change
if err != nil {
var notFoundError *cloudflare.NotFoundError
if errors.As(err, &notFoundError) {

jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
internal/provider/resource_cloudflare_dlp_profile.go Outdated
}

func resourceCloudflareDLPProfileImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
attributes := strings.SplitN(d.Id(), "/", 3)
Copy link
Member

Choose a reason for hiding this comment

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

i think you can do accountID/dlpProfileID for the import string instead. unless you're planning on supporting this at the zone level too?

jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
internal/provider/schema_dlp_profiles.go Outdated
Required: true,
ForceNew: true,
},
"id": {
Copy link
Member

Choose a reason for hiding this comment

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

id is a reserved keyword in Terraform schemas that is automatically handled; you don't need an entry for it.

jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
internal/provider/schema_dlp_profiles.go Outdated
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{DLPProfileTypeCustom, DLPProfileTypePredefined}, false),
Description: "The type of the profile - either 'predefined' or 'custom'.",
Copy link
Member

Choose a reason for hiding this comment

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

we can swap this to use the documentation helper and it will generate the correctly formatted output for the generated docs.

Suggested change
Description: "The type of the profile - either 'predefined' or 'custom'.",
Description: fmt.Sprintf("The type of the profile. %s", renderAvailableDocumentationValuesStringSlice([]string{DLPProfileTypeCustom, DLPProfileTypePredefined})),

jacobbednarz
jacobbednarz reviewed Oct 28, 2022
View reviewed changes
internal/provider/schema_dlp_profiles.go Outdated
@@ -0,0 +1,99 @@
package provider
Copy link
Member

Choose a reason for hiding this comment

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

let's rename this file for consistency too; schema_dlp_profiles.go -> schema_cloudflare_dlp_profiles.go

@jacobbednarz
Copy link
Member

i've taken a first pass over this and we've only got a few minor things to sort out.

to get the automatic documentation to generate (docs), you'll want to add examples for the resource + import (see #1957 for prior art). once you have that, run make docs locally to have run the generator.

cheers!

@jacobbednarz
Copy link
Member

pending getting the functionality enabled on the acceptance testing account. otherwise, we're looking good for now!

@jacobbednarz
Copy link
Member

acceptance tests are all passing now

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareDLPProfile_" -count 1 -parallel 1 -timeout 120m -parallel 1
?   	github.com/cloudflare/terraform-provider-cloudflare	[no test files]
=== RUN   TestAccCloudflareDLPProfile_Custom
--- PASS: TestAccCloudflareDLPProfile_Custom (9.01s)
=== RUN   TestAccCloudflareDLPProfile_Custom_MultipleEntries
--- PASS: TestAccCloudflareDLPProfile_Custom_MultipleEntries (8.73s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/provider	18.247s

@jacobbednarz jacobbednarz merged commit edebff1 into cloudflare:master Oct 31, 2022
@github-actions github-actions bot added this to the v3.27.0 milestone Oct 31, 2022
github-actions bot pushed a commit that referenced this pull request Oct 31, 2022
Update CHANGELOG.md for #1984
a585238
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

This functionality has been released in v3.27.0 of the Terraform Cloudflare 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 following the template. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobbednarz jacobbednarz jacobbednarz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.27.0
Development

Successfully merging this pull request may close these issues.
2 participants
@GreenStage @jacobbednarz