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

chore: run upstream provider-lint #4120

Merged
merged 9 commits into from
Jul 3, 2024
Merged

Conversation

corymhall
Copy link
Contributor

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 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
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

@corymhall corymhall requested review from t0yv0 and flostadler June 27, 2024 12:08
@@ -151,7 +151,7 @@ index 0000000000..a8bb57939e
+ actual, err := resourceLandingZoneNormalizeManifest(`
+ {
+ "governedRegions": [
+ "ap-southeast-2"
+ "REGION"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the upstream linter catch hardcoded regions/partitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, that is cool!

if flag {
+ //lintignore:S013
return map[string]*schema.Schema{
- names.AttrTags: &schema.Schema{Computed: tagsComputed},
Copy link
Contributor

Choose a reason for hiding this comment

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

That file is created in this patch. Imho we should correct it there, otherwise we'll make it harder to maintain that patch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh woops, I picked the wrong one! Good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@corymhall corymhall requested a review from flostadler June 27, 2024 15:44
Copy link
Contributor

@flostadler flostadler left a comment

Choose a reason for hiding this comment

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

nice!

@corymhall corymhall merged commit 426e13d into master Jul 3, 2024
25 checks passed
@corymhall corymhall deleted the corymhall/upstream-lint branch July 3, 2024 15:23
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v6.43.0.

corymhall added a commit that referenced this pull request 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run upstream linter after applying patches
4 participants