-
Notifications
You must be signed in to change notification settings - Fork 392
Terraform Provider for Custom Frameworks #2975
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
base: master
Are you sure you want to change the base?
Conversation
ccaf302
to
f8a90fe
Compare
docs/resources/custom_framework.md
Outdated
|
||
- `name` (String) The name of the requirement. | ||
|
||
Optional: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It auto generates as optional even though I have a validator in the schema requiring controls
ac03605
to
e2e496e
Compare
7da9031
to
e768276
Compare
data, _, err := r.Api.GetCustomFramework(r.Auth, state.Handle.ValueString(), state.Version.ValueString()) | ||
// If the framework does not exist, remove it from terraform state | ||
// This is to avoid the provider to return an error when the framework is deleted in the UI prior | ||
if err != nil && err.Error() == "400 Bad Request" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other scenarios that throw a 400? Can we case on something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For GetCustomFramework the other ways to get a 400 is if the handle or version is empty. But the terraform validator already checks that - so the only way to get a 400 if the framework doesn't exist
The framework wouldn't be in the state if it failed validation during Create so in Read it would have already been validated.
func (r *complianceCustomFrameworkResource) ImportState(ctx context.Context, request resource.ImportStateRequest, response *resource.ImportStateResponse) { | ||
// Split the ID into handle and version | ||
// The last hyphen separates handle and version | ||
lastHyphenIndex := strings.LastIndex(request.ID, "-") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we directly pass the handle and version instead of splitting the id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the only way to do it using the id. I can see how this can be a problem so I removed the import functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instead of importing the resource - because we can't directly pass in the handle and version I will change the create function to update the framework if the handle and version exists.
for _, requirement := range req.ConfigValue.Elements() { | ||
reqObj := requirement.(types.Object) | ||
name := reqObj.Attributes()["name"].(types.String).ValueString() | ||
log.Printf("Found requirement name in config: %s", name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add [INFO]
in front of the non-error logs
name = "requirement1" | ||
controls { | ||
name = "control1" | ||
rules_id = ["aaa-000-ccc", "bbb-000-ddd"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use real rule IDs here
response.State.RemoveResource(ctx) | ||
return | ||
} | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we switch the order of this and the if block on 182
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I can't switch the ordering because if I check if err is not nil without checking the status code first it would return before checking if the status code is 400
Motivation:
We wanted a terraform provider to interact with our Custom Framework APIs. The terraform resources will remain as the source of truth so whenever a resource is updated/created/deleted this will be reflected in the database and UI.
Testing
I built the Terrraform provider locally.
Tested the following:
Creating

Updating

Deleting/Destroying

Changing order of rules or requirements

no changes in state so no action taken
Commands:
also added unit tests!