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

(Onboarding) Implement MOCK-AZR-SVB-02 #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

samcolson4
Copy link

  • Expands the test to include a check for public_network_access_enabled
  • Sets the default for public_network_access_enabled to be false

@@ -35,6 +35,7 @@ func TestServiceBus(t *testing.T) {
// We verify that LocalAuthEnabled is always false
t.Run("MOCK-AZR-SVB-01", func(t *testing.T) {
assert.False(t, serviceBus.LocalAuthEnabled)
assert.False(t, serviceBus.PublicNetworkAccessEnabled)
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion here because it's the default value.. but what would happen if someone used it with a non-default (setting it to true)? Would this test still pass?

@gusfcarvalho
Copy link
Contributor

gusfcarvalho commented Nov 23, 2022

hi @samcolson4 ! Thanks for the PR!

Regarding the logic behind the implementation of SCF controls for terraform, if someone wants to deploy a a Service Bus with public access network enabled, they would be able to. However, from an enforcing layer, they would be blocked from actually running their code. Since the terraform module's idea is to allow people to run code that is aligned to the security controls, we should avoid that (having options that are, really, not options as they are blocked elsewhere).

Can you think of a way to implement these controls in order to make sure the terraform module user cannot set Public Network Access as enabled ? :)

@samcolson4
Copy link
Author

@gusfcarvalho - my latest commit uses the azurerm_servicebus_namespace_network_rule_set resource to define a (very simple) rule set on this. Does this satisfy the requirement?

I can see in Robel's implementation he added another layer to this around private subnets so I may grab him to speak about that too.

@samcolson4 samcolson4 added the do not merge This PR should not be merged label Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge This PR should not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants