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

linters: Update AWSR002 for default tags or replace with Semgrep check #18721

Open
gdavison opened this issue Apr 9, 2021 · 2 comments
Open
Labels
enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters.

Comments

@gdavison
Copy link
Contributor

gdavison commented Apr 9, 2021

The awsproviderlint check AWSR002 checks for the use of IgnoreAws() on the tags variable passed to d.Set("tags", ...). With support for default tags, the call the IgnoreAws() is typically no longer inline with the call to d.Set("tags", ...), so the check will fail.

Either update the check to validate that IgnoreAws() is called on the variable at some point prior to calling d.Set("tags", ...) or create a Semgrep rule to do the same.

The following code should pass the check

defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

tags, err := keyvaluetags.GlueListTags(glueConn, crawlerARN)

tags = tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig)

if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil {
	return fmt.Errorf("error setting tags: %w", err)
}
@gdavison gdavison added enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters. labels Apr 9, 2021
@bflad
Copy link
Member

bflad commented Apr 9, 2021

👍 Migrating this from go/analysis to semgrep since the checking needs to be contextually aware, which is hard to do correctly in AST-land and the lack of Go type matching shouldn't be too much of an issue.

@bflad
Copy link
Member

bflad commented Apr 9, 2021

Not sure if we want to bundle with this issue here, but we should also add other semgrep checking for in-flight contributions that would not have the default tags implementations, e.g.

  - id: UpdateContextFunc-GetChange
    fix: d.GetChange("tags_all")
    languages: [go]
    message: Update GetChange attribute in UpdateContextFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern: d.GetChange("tags")
      - pattern-inside: func $FUNCNAME(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

  - id: UpdateFunc-GetChange
    fix: d.GetChange("tags_all")
    languages: [go]
    message: Update GetChange attribute in UpdateFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern: d.GetChange("tags")
      - pattern-inside: func $FUNCNAME(d *schema.ResourceData, meta interface{}) error { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

  - id: UpdateContextFunc-HasChange
    fix: d.HasChange("tags_all")
    languages: [go]
    message: Update HasChange attribute in UpdateContextFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern: d.HasChange("tags")
      - pattern-inside: func $FUNCNAME(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

  - id: UpdateFunc-HasChange
    fix: d.HasChange("tags_all")
    languages: [go]
    message: Update HasChange attribute in UpdateFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern: d.HasChange("tags")
      - pattern-inside: func $FUNCNAME(d *schema.ResourceData, meta interface{}) error { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

  - id: UpdateContextFunc-HasChangesExcept
    fix: d.HasChangesExcept("tags", "tags_all")
    languages: [go]
    message: Update HasChange attribute in UpdateContextFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern-either:
        - pattern: d.HasChangeExcept("tags")
        - pattern: d.HasChangesExcept("tags")
      - pattern-inside: func $FUNCNAME(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

  - id: UpdateFunc-HasChangesExcept
    fix: d.HasChangesExcept("tags", "tags_all")
    languages: [go]
    message: Update HasChange attribute in UpdateFunc
    paths:
      exclude:
        - 'aws/resource_aws_inspector_resource_group.go'
      include:
        - 'aws/'
    patterns:
      - pattern-either:
        - pattern: d.HasChangeExcept("tags")
        - pattern: d.HasChangesExcept("tags")
      - pattern-inside: func $FUNCNAME(d *schema.ResourceData, meta interface{}) error { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^resource.*Update$"
    severity: WARNING

et al.

@anGie44 anGie44 self-assigned this Apr 20, 2021
@anGie44 anGie44 removed their assignment Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters.
Projects
None yet
Development

No branches or pull requests

3 participants