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 aws log collection service resource #437

Merged
merged 8 commits into from
Apr 29, 2020

Conversation

mhaley-miovision
Copy link
Contributor

The other half of #318. This resource allows the user to control which AWS services datadog will collect logs from. Related to https://github.com/terraform-providers/terraform-provider-datadog/pull/436.

@ghost ghost added the size/M label Mar 9, 2020
@ghost ghost added size/XL and removed size/L labels Mar 16, 2020
@gzussa
Copy link
Contributor

gzussa commented Apr 10, 2020

Quick update: We are looking at this PR and we are trying to make sure with the proper team that this implementation is the right way to do this resource moving forward. We will come back to you very very soon.

Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

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

💯 The core logic seems good to me. I have to request some minor changes changes. With those in we should be good to merge.

func resourceDatadogIntegrationAwsLogCollectionExists(d *schema.ResourceData, meta interface{}) (b bool, e error) {
// Exists - This is called to verify a resource still exists. It is called prior to Read,
// and lowers the burden of Read to be able to assume the resource exists.
client := meta.(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should cast the meta into a ProviderConfiguration like below, see other resources as example

providerConf := meta.(*ProviderConfiguration)
client := providerConf.CommunityClient

}

func resourceDatadogIntegrationAwsLogCollectionCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

dixit regarding the ProviderConfiguration

}

func resourceDatadogIntegrationAwsLogCollectionUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

dixit regarding the ProviderConfiguration

}

func resourceDatadogIntegrationAwsLogCollectionRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

dixit regarding the ProviderConfiguration


logCollections, err := client.GetIntegrationAWSLogCollection()
if err != nil {
return false, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the translateClientError method to sanitize client error messages. translateClientError(err, "error getting aws integration log collection"). See examples in other resources.


func checkIntegrationAWSLogCollectionDestroy(accProvider *schema.Provider) func(*terraform.State) error {
return func(s *terraform.State) error {
client := accProvider.Meta().(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

providerConf := accProvider.Meta().(*ProviderConfiguration)
client := providerConf.CommunityClient


func checkIntegrationAWSLogCollectionExists(accProvider *schema.Provider) func(*terraform.State) error {
return func(s *terraform.State) error {
client := accProvider.Meta().(*datadog.Client)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
client := accProvider.Meta().(*datadog.Client)
providerConf := accProvider.Meta().(*ProviderConfiguration)
client := providerConf.CommunityClient

if err != nil {
return err
}
for resourceType, r := range s.RootModule().Resources {
Copy link
Contributor

Choose a reason for hiding this comment

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

if err != nil {
return err
}
for _, r := range s.RootModule().Resources {
Copy link
Contributor

Choose a reason for hiding this comment

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

Helper method here for consistency as well :)

return false, nil
}

func prepareDatadogIntegrationAwsLogCollectionRequest(d *schema.ResourceData) datadog.IntegrationAWSServicesLogCollection {
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually name these method with the following convention buildSomethingStruct and we usually have it return a pointer.

func buildDatadogIntegrationAwsLogCollectionStruct(d *schema.ResourceData) *datadog.IntegrationAWSServicesLogCollection

@gzussa gzussa merged commit fada484 into DataDog:master Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants