-
Notifications
You must be signed in to change notification settings - Fork 14
feat: Add cloud posture custom resource and cloud rules data source #147
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: main
Are you sure you want to change the base?
Conversation
22e6889
to
ab438d4
Compare
ab438d4
to
6d6a396
Compare
|
||
"github.com/crowdstrike/gofalcon/falcon" | ||
cloudcompliance "github.com/crowdstrike/terraform-provider-crowdstrike/internal/cloud_compliance" | ||
"github.com/crowdstrike/terraform-provider-crowdstrike/internal/cloud_posture" |
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.
"github.com/crowdstrike/terraform-provider-crowdstrike/internal/cloud_posture" | |
cloudposture "github.com/crowdstrike/terraform-provider-crowdstrike/internal/cloud_posture" |
contentupdatepolicy.NewDefaultContentUpdatePolicyResource, | ||
contentupdatepolicy.NewContentUpdatePolicyPrecedenceResource, | ||
sensorvisibilityexclusion.NewSensorVisibilityExclusionResource, | ||
cloud_posture.NewCloudPostureCustomRuleResource, |
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.
cloud_posture.NewCloudPostureCustomRuleResource, | |
cloudposture.NewCloudPostureCustomRuleResource, |
sensorupdatepolicy.NewSensorUpdateBuildsDataSource, | ||
fcs.NewCloudAwsAccountsDataSource, | ||
contentupdatepolicy.NewContentCategoryVersionsDataSource, | ||
cloud_posture.NewCloudPostureRulesDataSource, |
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.
cloud_posture.NewCloudPostureRulesDataSource, | |
cloudposture.NewCloudPostureRulesDataSource, |
} | ||
|
||
type cloudComplianceFrameworkControlDataSourceModel struct { | ||
Controls []cloudComplianceFrameworkControlModel `tfsdk:"controls"` |
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.
We should use terraform types for our model struct. Almost every bug I have had to fix have been due to me using go types in my model. A recent example: #136
Controls []cloudComplianceFrameworkControlModel `tfsdk:"controls"` | |
Controls []types.Set `tfsdk:"controls"` |
You can use Schedule
in sensor update policy as reference.
data.Controls = types.SetNull(types.ObjectType{AttrTypes: cloudComplianceFrameworkControlModel{}.AttributeTypes()})
Basically I add a AttributeTypes
method to help reduce the boilerplate that is required to convert from go to tf types.
|
||
# retrieve all controls under a named benchmark | ||
data "crowdstrike_cloud_compliance_framework_controls" "all" { | ||
cloud_provider = "AWS" |
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.
This doesn't look like a valid option.
diags.AddError( | ||
"Failed to create rule.", | ||
fmt.Sprintf("Failed to create rule: %s", err), | ||
) |
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.
this error message needs to be updated
if len(queryPayload.Resources) == 0 { | ||
diags.AddWarning( | ||
"No Rules Found", | ||
"The query returned no rules. Please check your filter criteria.", | ||
) | ||
return nil, diags | ||
} |
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 don't think we need this warning.
diags.AddError( | ||
"Failed to get rule", | ||
fmt.Sprintf("Failed to get rule: %s", err), | ||
) |
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.
diags.AddError( | |
"Failed to get rule", | |
fmt.Sprintf("Failed to get rule: %s", err), | |
) | |
diags.AddError( | |
"Failed to get rules", | |
fmt.Sprintf("Failed to get rules: %s", err), | |
) |
if err = falcon.AssertNoError(getRulesPayload.Errors); err != nil { | ||
diags.AddError( | ||
"Failed to get rules", | ||
fmt.Sprintf("Failed to get rule: %s", err.Error()), |
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.
fmt.Sprintf("Failed to get rule: %s", err.Error()), | |
fmt.Sprintf("Failed to get rules: %s", err.Error()), |
rule.AttackTypes = types.SetValueMust(types.StringType, []attr.Value{}) | ||
for _, attackType := range resource.AttackTypes { | ||
rule.AttackTypes, diags = types.SetValue(types.StringType, append(rule.AttackTypes.Elements(), types.StringValue(attackType))) | ||
if diags.HasError() { | ||
return nil, diags | ||
} | ||
} |
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.
you should be able to do this instead
rule.AttackTypes = types.SetValueMust(types.StringType, []attr.Value{}) | |
for _, attackType := range resource.AttackTypes { | |
rule.AttackTypes, diags = types.SetValue(types.StringType, append(rule.AttackTypes.Elements(), types.StringValue(attackType))) | |
if diags.HasError() { | |
return nil, diags | |
} | |
} | |
attackTypeSet, diags := types.SetValueFrom(ctx, types.StringType, resource.AttackTypes) | |
if diags.HasError() { | |
return nil, diags | |
} |
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've added annotations and code change suggestions
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.
We need to add pagination support also for these get requests. I implement an example here
terraform-provider-crowdstrike/internal/content_update_policy/content_update_policy_precedence.go
Line 294 in 541081e
func (r *contentUpdatePolicyPrecedenceResource) getContentUpdatePoliciesByPrecedence( |
There are a few pagination methods our API's use so implementation varies.
if config.Benchmark.IsNull() && config.FQL.IsNull() { | ||
resp.Diagnostics.AddError( | ||
"Invalid Configuration", | ||
"At least one of 'benchmark' or 'fql' must be defined", | ||
) | ||
} |
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.
Is it possible for this endpoint to return everything? if it is then instead of requiring fql or benchmark, we should allow nothing to be provided, and just verify they aren't providing both are the same time.
No description provided.