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

Automation Account #39

Closed
CalvinRodo opened this issue Nov 1, 2021 · 9 comments · Fixed by #156
Closed

Automation Account #39

CalvinRodo opened this issue Nov 1, 2021 · 9 comments · Fixed by #156

Comments

@CalvinRodo
Copy link

The Automation Account that is created by the landing zone scripts fails the Audit Diagnostic Setting policy from the PBMM Controls.

It either doesn't have Logging or Metrics setup I haven't dug too much into it yet:
image

@SenthuranSivananthan
Copy link
Contributor

The condition for this policy is for both Logs & Metrics.

Snippet from the policy:

          "existenceCondition": {
            "allOf": [
              {
                "field": "Microsoft.Insights/diagnosticSettings/logs.enabled",
                "equals": "true"
              },
              {
                "field": "Microsoft.Insights/diagnosticSettings/metrics.enabled",
                "equals": "true"
              }
            ]
          }

As part of landing zone automation, we only enable Logs to avoid extra cost. Metrics will be part of Azure Monitor and doesn't need to be sent to Log Analytics Workspace.

Unfortunately, there's no way to disable the check on the metrics. We will file a feature request to Azure Policy/engineering team to see whether they can allow the customization.

@CalvinRodo
Copy link
Author

Yeah I dug into it right after and noticed it was for the Metrics. I'm filling out our 30 day cloud guardrail form and noticed that this was causing some of the audit policies to fail, I'll include a justification on my side for why it's failing when i fill out the guardrails document.

@SenthuranSivananthan
Copy link
Contributor

SenthuranSivananthan commented Nov 19, 2021

I've created a feature request via JimGBritt/AzurePolicy#41 to allow for customization at assignment time.

Workaround: You can add an Exemption to this policy through Azure Policy -> Exceptions.

The custom policy set "Custom - Log Analytics for Azure Services" contains a modified version of the built-in policy that only inspects logs.

image

@JimGBritt
Copy link
Member

I'm looking into this update on my side @SenthuranSivananthan but also realize it will be a breaking change for anyone using this in production already as a parameter will need to be added that provides the option between AuditIfNotExists and DeployIfNotExists. In addition, I want to review this from an option to allow this to be created as an optional path as opposed to the only path so that customers can migrate this direction if desired gradually.

My initial thought is introducing a parameter switch "-AuditIfNotExits" that will generate this option and it can be used. This then will allow the policy to be flipped when ready to move from Audit to Deploy (potentially). The beauty here is the script already provides the option for a unique Initiative name so you could deploy this twice - once for deploy and once for audit and all the policies will just inherit what the assignment is looking to apply. I'll look into this after my current update goes out hopefully soon.

                "type": "string",
                "defaultValue": "AuditIfNotExists",
                "allowedValues": [
                    "AuditIfNotExists",
                    "DeployIfNotExists",
                    "Disabled"
                ],
                "metadata": {
                    "displayName": "Effect",
                    "description": "The effect determines what happens when the policy rule is evaluated to match"
                }

@SenthuranSivananthan
Copy link
Contributor

Thanks @JimGBritt for looking into this. I think we also need the ability to choose which type of data to audit/deploy. Today, the Audit diagnostic settings policy expects both Metrics & Logs to be enabled. It will be helpful to have this be toggled.

This is the existence condition:

          "existenceCondition": {
            "allOf": [
              {
                "field": "Microsoft.Insights/diagnosticSettings/logs.enabled",
                "equals": "true"
              },
              {
                "field": "Microsoft.Insights/diagnosticSettings/metrics.enabled",
                "equals": "true"
              }
            ]
          }

To avoid breaking changes, I think the default value for these parameters can be true so they enable both data streams.

This would also have be changed at the Initiatives like Canada Federal PBMM or HIPAA/HITRUST that needs to be updated too with similar parameters & defaults.

@JimGBritt
Copy link
Member

JimGBritt commented Nov 24, 2021

Enable logs and metrics are already parameters so not sure there will be much to do there but will review.

@SenthuranSivananthan
Copy link
Contributor

FYI this is the built-in policy for reference:

image

@JimGBritt
Copy link
Member

@SenthuranSivananthan understood. Also for reference - if we are talking about updating my script to add an effect of AuditIfNotExists and having enabled (true/false) that is already in my artifact export [see below]

image

My thought was to just add in a parameter for effect (AuditIfNotExists/DeployIfNotExists/Disabled) so that there is an option to toggle. Should be trivial.

@SenthuranSivananthan
Copy link
Contributor

Azure Policy engineering team has updated PBMM Policy Initiative (policy set) so that diagnostic settings are configurable to enable/disable both metrics & logs. We will update the landing zone automation to accommodate shortly.

image

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