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

feat: create global 'deny' rule when more narrow scoped rules are created by the module #396

Merged
merged 21 commits into from
Mar 18, 2024

Conversation

Aashiq-J
Copy link
Member

@Aashiq-J Aashiq-J commented Jan 30, 2024

Description

Issue : #388

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content
  • minimum required provider version is 1.62.0.
  • Ability to scope a rule per region.
  • Support for multiple attributes per rule for a service.
  • Remove public default context set to 1.1.1.1
  • 0 context rule for services by default, which will deny all requests made to a service. (Note: By default enforcement mode is set to report-only).
  • option create a global 'deny' rule for all the scoped rule for a service. By default it is set to true.

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

3 similar comments
@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J Aashiq-J requested a review from vburckhardt January 31, 2024 04:23
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 1, 2024

Adding ignoreUpdates because its just a update-in-place.
Screenshot 2024-02-01 at 11 27 55 AM

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 1, 2024

Added the feature to pass multiple attribute in this PR.

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 1, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 5, 2024

/run pipeline

2 similar comments
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 5, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 5, 2024

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

I am not sure, but maybe those upgrade issues can be overcome.

modules/fscloud/main.tf Outdated Show resolved Hide resolved
tests/pr_test.go Outdated Show resolved Hide resolved
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 7, 2024

/run pipeline

2 similar comments
@shemau
Copy link
Contributor

shemau commented Feb 7, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Feb 7, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@iamar7 iamar7 mentioned this pull request Mar 1, 2024
6 tasks
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 5, 2024

/run pipeline

@vburckhardt
Copy link
Member

vburckhardt commented Mar 5, 2024

for this one "Default 1.1.1.1 context is not needed now as all Cloud services now have this implicit deny." -> have we actually checked it. I've been getting inconsistent answers from services, so I would suggest to actually test this out.

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 6, 2024

Tested IKS rule with 0 context scoping to instance id, region and all the resources. Unable to access the cluster when the rule was in-place.
image
image
image

Also tested the same for VPC, created rule with 0 context scoping to a region.
image
image

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 6, 2024

The implicit deny feature is only available in the new provider version which is in beta stage.

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 6, 2024

/run pipeline

examples/fscloud/version.tf Show resolved Hide resolved
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 7, 2024

/run pipeline

2 similar comments
@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 7, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

Aashiq-J commented Mar 7, 2024

/run pipeline

@Aashiq-J Aashiq-J requested a review from vburckhardt March 7, 2024 12:15
@Ak-sky
Copy link
Member

Ak-sky commented Mar 7, 2024

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

This is a very new version of the provider. So new in fact that many consumers will have to upgrade. IMHO it would be good to put a note and reason (what was fixed) in the release notes section of the PR. These seems particularly important because so many modules support CBR and thus implicitly require 1.62.0.

It may be this feature to allow global deny? terraform-provider-ibm pull 5058.

Any example in our modules that is pinned to a lower level will immediately fail and require updating.

@Aashiq-J
Copy link
Member Author

/run pipeline

@Aashiq-J
Copy link
Member Author

updated the release notes.

@toddgiguere toddgiguere dismissed vburckhardt’s stale review March 18, 2024 12:58

Requested changes in review have been resolved.

@toddgiguere toddgiguere merged commit 512a33b into main Mar 18, 2024
2 checks passed
@toddgiguere toddgiguere deleted the global-deny branch March 18, 2024 13:04
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fscloud module: create global 'deny' rule when more narrow scoped rules are created by the module
6 participants