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

Run upstream linter after applying patches #4110

Closed
mjeffryes opened this issue Jun 25, 2024 · 1 comment · Fixed by #4120
Closed

Run upstream linter after applying patches #4110

mjeffryes opened this issue Jun 25, 2024 · 1 comment · Fixed by #4120
Assignees
Labels
kind/engineering Work that is not visible to an external user
Milestone

Comments

@mjeffryes
Copy link
Member

As reported in Improper partition used when generating SNS Topic ARN· pulumi-aws/3926, one of our local patches of the upstream TF provider introduced a hardcoded partition into the ARN, which prevented the resource from working with US Gov or China aws partitions. There's actually a linter in the upstream TF provider (https://github.com/hashicorp/terraform-provider-aws/tree/main/.ci/providerlint) with a rule for this exact problem (https://github.com/hashicorp/terraform-provider-aws/tree/main/.ci/providerlint/passes/AWSAT005).

As a remediation, we'd like to start running the upstream linter on upstream after our patches to hopefully catch other problems like this in the future.

@mjeffryes mjeffryes added the kind/engineering Work that is not visible to an external user label Jun 25, 2024
@corymhall corymhall self-assigned this Jun 26, 2024
corymhall added a commit that referenced this issue Jun 26, 2024
This adds a step for running the upstream `provider-lint` make target.

As part of this I had to fix some of the patches which violated some
lint rules.

**0009-Add-ECR-credentials_data_source.patch**
- `ForceNew` does not apply to data sources

**0032-DisableTagSchemaCheck-for-PF-provider.patch**
- Schema have to have a `Type`
- Also needed to add a ignore for `S013` which forces `Computed`,
  `Optional` or `Required` to be set. Looks like it can't recognize the
  `tagsComputed` var

**0034-Fail-fast-when-PF-resources-are-dropped.patch**
- Added a lint ignore for a rule which doesn't allow panics

**0050-Normalize-retentionDays-in-aws_controltower_landing_.patch**
- This test doesn't actually need a region or partition so replacing
  with a placeholder

closes #4110
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #4120 and shipped in release v6.43.0.

corymhall added a commit that referenced this issue Jul 11, 2024
# This is the 1st commit message:

Fix import resources with provider default tags

We have special logic around applying default provider tags to
resources. This logic only applied to the `Check` call which means it
was not applied when you were importing resources. This PR extends that
logic to also run during the `Read` call.

fix #4030, fix 4080

# This is the commit message #2:

skip test

# This is the commit message #3:

fixing test

# This is the commit message #4:

Adding more tests

# This is the commit message #5:

Upgrade pulumi-terraform-bridge to v3.86.0 (#4160)

This PR was generated via `$ upgrade-provider pulumi/pulumi-aws
--kind=bridge --pr-reviewers=guineveresaenger`.

Fixes #4091
Fixes #4137

---

- Upgrading pulumi-terraform-bridge from v3.85.0 to v3.86.0.
- Upgrading pulumi-terraform-bridge/pf from v0.38.0 to v0.39.0.
# This is the commit message #6:

chore: run upstream provider-lint (#4120)

This adds a step for running the upstream `provider-lint` make target.

As part of this I had to fix some of the patches which violated some
lint rules.

**0009-Add-ECR-credentials_data_source.patch**
- `ForceNew` does not apply to data sources

**0032-DisableTagSchemaCheck-for-PF-provider.patch**
- Schema have to have a `Type`
- Also needed to add a ignore for `S013` which forces `Computed`,
  `Optional` or `Required` to be set. Looks like it can't recognize the
  `tagsComputed` var

**0034-Fail-fast-when-PF-resources-are-dropped.patch**
- Added a lint ignore for a rule which doesn't allow panics

**0050-Normalize-retentionDays-in-aws_controltower_landing_.patch**
- This test doesn't actually need a region or partition so replacing
  with a placeholder

closes #4110
# This is the commit message #7:

fix: CVE-2024-24791 (#4175)

Fixes #4163

Upgrades minimally required Go versions to those unaffected by
CVE-2024-24791.
@mjeffryes mjeffryes added this to the 0.107 milestone Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/engineering Work that is not visible to an external user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants