hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.72k stars 9.08k forks source link

[Bug]: Data Source: aws_ssm_patch_baseline - "no results" false positive #33953

Open n2N8Z opened 10 months ago

n2N8Z commented 10 months ago

Terraform Core Version

1.5.0

AWS Provider Version

5.21.0

Affected Resource(s)

data aws_ssm_patch_baseline

Expected Behavior

data aws_ssm_patch_baseline returns matching results same as aws cli describe-patch-baselines

Actual Behavior

data aws_ssm_patch_baseline requires either default_baseline to be defined, or operating_system to be defined AND match. default_baseline and operating_system are both listed as optional.

Relevant Error/Panic Output Snippet

No errors.

Terraform Configuration Files

provider aws {
  region = "eu-west-1"
}

data aws_ssm_patch_baseline baseline {
  owner            = "AWS"
  name_prefix      = "AWS-WindowsPredefinedPatchBaseline-OS-Applications"
}

output debug {
    value = data.aws_ssm_patch_baseline.baseline.id
}

Steps to Reproduce

see config above

aws cli equivalent:

$ aws ssm describe-patch-baselines --filters Key=OWNER,Values=AWS Key=NAME_PREFIX,Values=AWS-WindowsPredefinedPatchBaseline-OS-Applications
{
    "BaselineIdentities": [
        {
            "BaselineId": "arn:aws:ssm:eu-west-1:845259048710:patchbaseline/pb-06e9f0d7e54690025",
            "BaselineName": "AWS-WindowsPredefinedPatchBaseline-OS-Applications",
            "OperatingSystem": "WINDOWS",
            "BaselineDescription": "For the Windows Server operating system, approves all patches that are classified as CriticalUpdates or SecurityUpdates and that have an MSRC severity of Critical or Important. For Microsoft applications, approves all patches. Patches are auto-approved seven days after release.",
            "DefaultBaseline": false
        }
    ]
}

Debug Output

No response

Panic Output

No response

Important Factoids

https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/ssm/patch_baseline_data_source.go - The problem occurs because operating_system and default_baseline are applied as filters on returned results. operating_system is a filter supported by the API and hence it's inefficient to apply it to returned results instead. default_baseline has unexpected results, because a "true" match breaks out of the loop, and a "false" value never passes the "if" test since GetOk returns false, false.

I would have gone for a fix similar to the below, but I'm not sure about the strange default_baseline handling, whether it has some reason for it's strangeness:

diff --git a/internal/service/ssm/patch_baseline_data_source.go b/internal/service/ssm/patch_baseline_data_source.go
index 5ae469620a..d44f4b15e8 100644
--- a/internal/service/ssm/patch_baseline_data_source.go
+++ b/internal/service/ssm/patch_baseline_data_source.go
@@ -175,6 +175,15 @@ func dataPatchBaselineRead(ctx context.Context, d *schema.ResourceData, meta int
                })
        }

+       if v, ok := d.GetOk("operating_system"); ok {
+               filters = append(filters, &ssm.PatchOrchestratorFilter{
+                       Key: aws.String("OPERATING_SYSTEM"),
+                       Values: []*string{
+                               aws.String(v.(string)),
+                       },
+               })
+       }
+
        params := &ssm.DescribePatchBaselinesInput{
                Filters: filters,
        }
@@ -188,20 +197,15 @@ func dataPatchBaselineRead(ctx context.Context, d *schema.ResourceData, meta int
        }

        var filteredBaselines []*ssm.PatchBaselineIdentity
-       if v, ok := d.GetOk("operating_system"); ok {
-               for _, baseline := range resp.BaselineIdentities {
-                       if v.(string) == aws.StringValue(baseline.OperatingSystem) {
-                               filteredBaselines = append(filteredBaselines, baseline)
-                       }
-               }
-       }
-
-       if v, ok := d.GetOk("default_baseline"); ok {
-               for _, baseline := range filteredBaselines {
+       for _, baseline := range resp.BaselineIdentities {
+               v, ok := d.GetOk("default_baseline")
+               if v, ok := d.GetOk("default_baseline"); ok {
                        if v.(bool) == aws.BoolValue(baseline.DefaultBaseline) {
                                filteredBaselines = []*ssm.PatchBaselineIdentity{baseline}
                                break
                        }
+               } else {
+                       filteredBaselines = append(filteredBaselines, baseline)
                }
        }

References

No response

Would you like to implement a fix?

No

github-actions[bot] commented 10 months ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue