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

fix: Deploy ASIM parsers without redeploying the Log Analytics workspace #11299

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

frendsick
Copy link

Change(s):

  • Deploy ASIM parsers without redeploying the Log Analytics workspace

Reason for Change(s):

  • Each parser deployment ARM template deploys the Log Analytics workspace before deploying the saved search. A conflict occurs when tens of parser deployment ARM templates try to deploy the Log Analytics workspace at the same time.
  • The ASIM parser deployment ARM template should only install saved searches in an already existing Log Analytics workspace. Creating a new one could be unwanted behavior.
    • After these changes, the deployment will fail with a ResourceNotFound error code if the Log Analytics workspace given as a parameter does not exist.
  • Resolves Manually ASIM Deployment - Failed to validate, Conflict #8623

Version Updated:

  • No

Testing Completed:

  • I tested installing the ASIM parser collections ASimAuditEvents, ASimAuthentication, ASimDhcpEvent, ASimDns, ASimFileEvent, ASimNetworkSession, ASimProcessEvent, ASimRegistryEvent, ASimUserManagement, and ASimWebSession by changing the templateLink keys to point to the fixed files in my forked repository. The installations succeeded without any conflicts, whereas the old parser collection ARM templates always had some conflicts.
    • I could not test the ASimFullDeployment.json as it points to the intermediate ASIM parser collections in the Azure/Azure-Sentinel repository.

Checked that the validations are passing and have addressed any issues that are present:

  • The ASIM parser collections ARM templates worked fine after this change

How the changes were made?

I modified each of the ARM templates with the following shell script, which does the JSON editing using jq. Both the fix-asim-parsers.sh and fix-asim-arm.jq scripts were located in the root directory of the Azure-Sentinel repository.

fix-asim-parsers.sh

#!/bin/sh

fix_arm_template() {
  temp_file="temp.json"
  jq -f ./fix-asim-arm.jq $1 > $temp_file
  mv $temp_file $1
  echo "Fixed $1"
}

for parser_folder in Parsers/ASim*; do
  # This condition checks if the file starts with "ASim " (with a space)
  if [ "${parser_folder#Parsers/ASim }" != "$parser_folder" ]; then
    continue
  fi

  for arm_folder in $parser_folder/ARM/*; do
    [ -d "$arm_folder" ] &&
      fix_arm_template "$arm_folder/$(basename $arm_folder).json"
  done
done

fix-asim-arm.jq:

{
  "$schema": ."$schema",
  "contentVersion": .contentVersion,
  "parameters": .parameters,
  "resources": [
    {
      "type": "Microsoft.OperationalInsights/workspaces/savedSearches",
      "apiVersion": "2020-08-01",
      "name": "[concat(parameters('Workspace'), '/\(.resources[0].resources[0].name)')]",
      "location": .resources[0].location,
      "properties": (
        .resources[0].resources[0].properties | del(.dependsOn)
      )
    }
  ]
}

Screenshot of testing installation of the ASimRegistryEvent parser collection:

image

The deployment will fail with `ResourceNotFound` error code if the Log Analytics workspace does not exist.
@frendsick frendsick requested review from a team as code owners October 18, 2024 13:53
@frendsick
Copy link
Author

@microsoft-github-policy-service agree company="Loihde Trust Oy"

@vakohl
Copy link
Contributor

vakohl commented Oct 21, 2024

@frendsick thanks for helping fix this issue.

Updating the ARM templates won't fix the issue as ARM templates get re-generated on every parser update.
The following scripts does this logic:
https://github.com/Azure/Azure-Sentinel/blob/master/.script/kqlFuncYaml2Arm.ps1
https://github.com/Azure/Azure-Sentinel/blob/master/ASIM/dev/ASimYaml2ARM/KqlFuncYaml2Arm.py

I guess, the logic you are suggesting, we may need to apply in underlying script as well. I'll try reviewing these changes in this week.

@frendsick
Copy link
Author

@vakohl Thank you for explaining the architecture.

The KqlFuncYaml2Arm.py script uses func_arm_template.json as base for the ARM-template generation. Thus, we probably need to change the schema to the func_arm_template.json and to the KqlFuncYaml2Arm.py script to match with my proposed schema, that only installs the saved search without installing the Log Analytics workspace first.

Does my assumptions sound correct? Would you like me to tackle this change also?

@vakohl
Copy link
Contributor

vakohl commented Oct 21, 2024

@vakohl Thank you for explaining the architecture.

The KqlFuncYaml2Arm.py script uses func_arm_template.json as base for the ARM-template generation. Thus, we probably need to change the schema to the func_arm_template.json and to the KqlFuncYaml2Arm.py script to match with my proposed schema, that only installs the saved search without installing the Log Analytics workspace first.

Does my assumptions sound correct? Would you like me to tackle this change also?

@frendsick that would be great if you got time to make that change. I'll review and test the changes you suggest,

@frendsick
Copy link
Author

Are my original ARM template modifications wanted at all, as they should be generated by the kqlFuncYaml2Arm.ps1 script? That PowerShell just will not trigger because none of the files in $($PSScriptRoot)/../Parsers/$($schema)/Parsers are changed by this PR, so we probably have to update the ARM templates anyway manually.

@vakohl
Copy link
Contributor

vakohl commented Oct 21, 2024

we would need to fix [current ARM template] + [Scripts to handle future changes]

@frendsick
Copy link
Author

Do we need to change the ARM templates for the other parsers rather than the ASIM parsers? In my opinion, this PR should only affect the ASIM parsers. Then, after this PR is merged, you can use the modified kqlFuncYaml2Arm.ps1 to regenerate ARM templates wherever needed.

@vakohl
Copy link
Contributor

vakohl commented Oct 21, 2024

Do we need to change the ARM templates for the other parsers rather than the ASIM parsers? In my opinion, this PR should only affect the ASIM parsers. Then, after this PR is merged, you can use the modified kqlFuncYaml2Arm.ps1 to regenerate ARM templates wherever needed.

I think the script is only used for ASIM parsers

ASIM parser installation ARM templates should only install the saved
search if the Log Analytics workspace already exists.

Relates: Azure#8623
Using fixed version of `func_arm_template.json` from dd86c48

The following command was used to generate ARM templates for each ASIM parser schema,
where `$SCHEMA` was set first as `ASimAuditEvent`, then `ASimAuthentication`, etc.:

`python3 ASIM/dev/ASimYaml2ARM/KqlFuncYaml2Arm.py -m asim -d Parsers/$SCHEMA/ARM Parsers/$SCHEMA/Parsers`
@frendsick
Copy link
Author

frendsick commented Oct 22, 2024

@vakohl I made the required changes to func_arm_template.json and KqlFuncYaml2Arm.py. Then, I regenerated ARM templates for all ASIM schemas to verify that KqlFuncYaml2Arm.py generates files in the expected format.

The KqlFuncYaml2Arm.py script has not been automatically triggered for some newer parser schemas, like ASimAuditEventIllumioSaaSCore. kqlFuncYaml2Arm.ps1 script only runs KqlFuncYaml2Arm.py to files that are changed; The getModifiedASimSchemas.ps1 uses git diff to check when the ARM templates have to be regenerated in such a way that the new parser files are not handled automatically.

The only difference between my previous JQ script and the new version of KqlFuncYaml2Arm.py is that the Python script inserts a new line to the end of the ARM template file. Since that does not affect functionality, I did not update the ARM template files, which only changed the new line at EOF.

I verified that the changes still work by reinstalling the ASimAuditEvent schemas. The old versions do not have to be removed; the "etag": "*" definition automatically overrides older versions with the new ones.

@frendsick
Copy link
Author

The PR is ready for review.

@frendsick
Copy link
Author

@vakohl The most important changes are made to the KqlFuncYaml2Arm.py and func_arm_template.json files. The rest are generated changes to ARM templates.

You can verify if the changes work by running the KqlFuncYaml2Arm.py script for each parser collection like kqlFuncYaml2Arm.ps1 does:

# Run the command from the root folder of the repository
# $schema => Parser collection, like ASimAuthentication or ASimAuditEvent
python3 ASIM/dev/ASimYaml2ARM/KqlFuncYaml2Arm.py -m asim -d Parsers/$schema/ARM Parsers/$schema/Parsers

The generated ARM template files should look the same as the changed files in this PR. You should only see newline changes at the end of files, which do not affect functionality. You can verify if the files match otherwise by using the git diff --ignore-space-at-eol command.

@frendsick
Copy link
Author

I resolved the merge conflicts by copying the current query version from the master branch and keeping the changes from this branch. This way, the ARM templates will be in the same state as they would be if they were generated from the master branch using this PRs version of KqlFuncYaml2Arm.py.

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.

Manually ASIM Deployment - Failed to validate, Conflict
3 participants