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

Update tier3.bicep Adding Defender configuration to Tier 3 #725

Merged

Conversation

LManning-Dev
Copy link
Contributor

@LManning-Dev LManning-Dev commented Jul 18, 2022

Description

Added Defender Configuration used in mlz.bicep into the tier 3

Issue reference

The issue this PR will close: #723

Checklist

Please make sure you've completed the relevant tasks for this PR out of the following list:

  • All acceptance criteria in the backlog item are met
  • The documentation is updated to cover any new or changed features
  • Manual tests have passed
  • Relevant issues are linked to this PR

@LManning-Dev LManning-Dev requested a review from a team as a code owner July 18, 2022 21:53
@lisamurphy-msft
Copy link
Contributor

Thank you for updating this. I propose that you also update the README located here to include the additional optional parameter you are proposing be added to the Tier3 deployment.

@LManning-Dev
Copy link
Contributor Author

Will do. Thank you

Pulling Defender info from the core mlz.bicep deployment
Updating Tier 3 Readme
Fixing error with duplicate parameters. Adding Moving description comments to the parameters that pull from mlzDeploymentVariables
@LManning-Dev
Copy link
Contributor Author

Thank you for updating this. I propose that you also update the README located here to include the additional optional parameter you are proposing be added to the Tier3 deployment.

@lisamurphy-msft
I added the ability to pull the security option from the core (So it defaults to the same as the core but is overwritable) and added the variables to the readme. I think all 3 commits are showing up here.

Thank you for your time and help with this!

Note: Out of the 24 original variables, 22 are not in the readme. Should that be brought up in a different issue?

Parameters in the ReadMe
param resourcePrefix string
param virtualNetworkAddressPrefix string = '10.0.125.0/26'

Parameters not in the Readme
param resourceSuffix string = 'mlz'
param location string = deployment().location
param workloadSubscriptionId string = subscription().subscriptionId
param mlzDeploymentVariables object = json(loadTextContent('../deploymentVariables.json'))
param hubSubscriptionId string = mlzDeploymentVariables.hub.Value.subscriptionId
param hubResourceGroupName string = mlzDeploymentVariables.hub.Value.resourceGroupName
param hubVirtualNetworkName string = mlzDeploymentVariables.hub.Value.virtualNetworkName
param hubVirtualNetworkResourceId string = mlzDeploymentVariables.hub.Value.virtualNetworkResourceId
param logAnalyticsWorkspaceResourceId string = mlzDeploymentVariables.logAnalyticsWorkspaceResourceId.Value
param logAnalyticsWorkspaceName string = mlzDeploymentVariables.logAnalyticsWorkspaceName.Value
param firewallPrivateIPAddress string = mlzDeploymentVariables.firewallPrivateIPAddress.Value
param deployDefender bool = mlzDeploymentVariables.deployDefender.Value
param emailSecurityContact string = mlzDeploymentVariables.emailSecurityContact.Value
param virtualNetworkDiagnosticsLogs array = []
param virtualNetworkDiagnosticsMetrics array = []
param networkSecurityGroupRules array = []
param networkSecurityGroupDiagnosticsLogs array
param networkSecurityGroupDiagnosticsMetrics array = []
param subnetAddressPrefix string = '10.0.125.0/27'
param subnetServiceEndpoints array = []
param logStorageSkuName string = 'Standard_GRS'
param tags object = {}

Copy link
Contributor

@lisamurphy-msft lisamurphy-msft left a comment

Choose a reason for hiding this comment

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

Please address the comment included; this will slightly change the outputs listed in the documentation for a base MLZ deployment. While this is not a breaking change, it should be fixed to ensure accuracy.

We are adding additional outputs to the MLZ baseline; this is to incorporate adding in outputs for the deploymentVariables.json for Tier3.

While this isn't a big change, we should probably ensure that the outputs listed in the README.md of the MissionLZ bicep code include the outputs linked in the outputs documentation

Please add to the list:

deployDefender.value
emailSecurityContact.value

@lisamurphy-msft
Copy link
Contributor

Failing the Linting check; will investigate.
Other then that I don't see any other issues preventing merging.

@lisamurphy-msft lisamurphy-msft dismissed their stale review August 19, 2022 19:06

need to address linting errors

lisamurphy-msft and others added 5 commits August 24, 2022 12:22
… they are inherited

With the inheritance pattern proposed in this PR, this will not be necessary to explicitly state here. The state of just using the same default configuration in the original MLZ deployment will be sufficient with the added changes of documenting the additional two outputs.
@lisamurphy-msft
Copy link
Contributor

Thank you for updating as requested; this looks good!
Merging

@lisamurphy-msft lisamurphy-msft enabled auto-merge (squash) September 1, 2022 22:16
src/bicep/add-ons/tier3/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tier 3 Missing Defender Implementation
2 participants