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

JSON Parse gives a strange warning and causes GitHub Super Linter to fail and can't be skipped #487

Closed
spoelly opened this issue Aug 4, 2021 · 17 comments
Labels
bug Something isn't working
Milestone

Comments

@spoelly
Copy link
Contributor

spoelly commented Aug 4, 2021

Back from vacation we see that the ARM-TTK linter is not working fine in our Super Linter execution.
After running the ARM-TTK locally we see this strange warning for one of our templates:

VERBOSE: Conversion from JSON failed with error: After parsing a value an unexpected character was encountered: o. Path 'variables.kvtAccessPoliciesArray', line 118, position 97.

After some debugging we see that line 118 is not the issue but this one:

'''
"mergeKvtAccessPoliciesCondition": "[not(contains(string(parameters('kvtAccessPolicies')),concat('"objectId":"',parameters('kvtObjectId'),'"')))]",
'''

Very strange because the json is valid otherwise the deployment of our template would fail. So as a work-arround we tried to skip the "JSONFiles Should Be Valid" check but this is imposible. :(

This is our JSON file

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "parameters": {
    "kvtName": {
      "type": "string",
      "metadata": {
        "description": "Name of the new Keyvault"
      }
    },
    "kvtSkuName": {
      "type": "string",
      "defaultValue": "Standard",
      "allowedValues": [
        "Standard",
        "Premium"
      ],
      "metadata": {
        "description": "SKU for the vault"
      }
    },
    "kvtEnabledForDeployment": {
      "type": "bool",
      "defaultValue": false,
      "metadata": {
        "description": "Set TRUE to enable access for deployments"
      }
    },
    "kvtEnabledForTemplateDeployment": {
      "type": "bool",
      "defaultValue": false,
      "metadata": {
        "description": "Set TRUE to enable access for Template deployments"
      }
    },
    "kvtEnabledForDiskEncryption": {
      "type": "bool",
      "defaultValue": false,
      "metadata": {
        "description": "Set TRUE to enable access for disk encryption"
      }
    },
    "kvtObjectId": {
      "type": "string",
      "defaultValue": "",
      "metadata": {
        "description": "ObjectId of Pipeline account"
      }
    },    
    "kvtNetworkAclsBypass": {
      "type": "string",
      "defaultValue": "None",
      "allowedValues": [
        "None",
        "AzureServices"
      ],
      "metadata": {
        "description": "Tells what traffic can bypass the network rules"
      }
    },
    "kvtAccessPolicies": {
      "type": "array",
      "defaultValue": [],
      "metadata": {
        "description": "Access policies defined for the new vault"
      }
    },
    "kvtIpRules": {
      "type": "array",
      "defaultValue": [],
      "metadata": {
        "description": "An array with IPRule objects"
      }
    },
    "kvtVirtualNetworkRules": {
      "type": "array",
      "defaultValue": [],
      "metadata": {
        "description": "An array with VirtualNetworkRule objects"
      }
    },
    "location": {
      "type": "string",
      "defaultValue": "[resourceGroup().location]",
      "metadata": {
        "description": "Location of the resource deployment"
      }
    }
  },
  "variables": {    
    "kvtDefaultAccessPolicies": [
      {
        "tenantId": "[subscription().tenantId]",
        "objectId": "[parameters('kvtObjectId')]",
        "permissions": {
          "keys": [
            "Backup",
            "Create",
            "Delete",
            "Get",
            "Import",
            "List",
            "Restore",
            "Update",
            "Recover"
          ],
          "secrets": [
            "All"
          ],
          "certificates": [
            "All"
          ]
        }
      }
    ],
    "kvtDefaultAccessPoliciesArray": "[if(empty(parameters('kvtObjectId')),json('[]'),variables('kvtDefaultAccessPolicies'))]",
    "mergeKvtAccessPoliciesCondition": "[not(contains(string(parameters('kvtAccessPolicies')),concat('\"objectId\":\"',parameters('kvtObjectId'),'\"')))]",
    "kvtAccessPoliciesArray": "[if(variables('mergeKvtAccessPoliciesCondition'),union(variables('kvtDefaultAccessPoliciesArray'),parameters('kvtAccessPolicies')),parameters('kvtAccessPolicies'))]",
    "azureServicesBypassCondition": "[or(or(parameters('kvtEnabledForDeployment'),parameters('kvtEnabledForTemplateDeployment')),parameters('kvtEnabledForDiskEncryption'))]",
    "kvtNetworkAclsCondition": "[not(and(empty(parameters('kvtIpRules')),empty(parameters('kvtVirtualNetworkRules'))))]"
  },
  "resources": [
    {
      "name": "[parameters('kvtName')]",
      "type": "Microsoft.KeyVault/vaults",
      "apiVersion": "2021-04-01-preview",
      "location": "[parameters('location')]",
      "properties": {
        "accessPolicies": "[if(empty(parameters('kvtAccessPolicies')),variables('kvtDefaultAccessPoliciesArray'),variables('kvtAccessPoliciesArray'))]",
        "enabledForDeployment": "[parameters('kvtEnabledForDeployment')]",
        "enabledForDiskEncryption": "[parameters('kvtEnabledForDiskEncryption')]",
        "enabledForTemplateDeployment": "[parameters('kvtEnabledForTemplateDeployment')]",
        "enableSoftDelete": true,
        "sku": {
          "name": "[parameters('kvtSkuName')]",
          "family": "A"
        },
        "tenantId": "[subscription().tenantId]",
        "networkAcls": {
          "bypass": "[if(variables('azureServicesBypassCondition'),'AzureServices',parameters('kvtNetworkAclsBypass'))]",
          "defaultAction": "[if(variables('kvtNetworkAclsCondition'),'Deny','Allow')]",
          "ipRules": "[parameters('kvtIpRules')]",
          "virtualNetworkRules": "[parameters('kvtVirtualNetworkRules')]"
        }
      }
    }
  ],
  "outputs": {
    "kvtName": {
      "type": "string",
      "value": "[parameters('kvtName')]"
    },
    "kvtResourceId": {
      "type": "string",
      "value": "[resourceId('Microsoft.KeyVault/vaults', parameters('kvtName'))]"
    },
    "kvtUri": {
      "type": "string",
      "value": "[reference(parameters('kvtName')).VaultUri]"
    },
    "vaults": {
      "type": "array",
      "value": [
        {
          "Name": "[parameters('kvtName')]",
          "ResourceId": "[resourceId('Microsoft.KeyVault/vaults', parameters('kvtName'))]",
          "Uri": "[reference(parameters('kvtName')).VaultUri]"
        }
      ]
    }
  }
}

This is what we get running arm-ttk locally with verbose


VERBOSE: Conversion from JSON failed with error: After parsing a value an unexpected character was encountered: o. Path 'variables.kvtAccessPoliciesArray', line 118, position 97.                                                                                                                                                                                      

Validating templates\azuredeploy.json                                                                                     
AllFiles                                                                                                                  
    [+] JSONFiles Should Be Valid (28 ms)                                                                                 
deploymentTemplate                                                                                                        
    [+] adminUsername Should Not Be A Literal (14 ms)                                                                       
    [+] apiVersions Should Be Recent In Reference Functions (11 ms)                                                         
    [+] apiVersions Should Be Recent (14 ms)                                                                                
    [+] artifacts parameter (11 ms)                                                                                        
    [+] CommandToExecute Must Use ProtectedSettings For Secrets (27 ms)                                                    
    [+] DependsOn Best Practices (10 ms)                                                                                   
    [+] Deployment Resources Must Not Be Debug (10 ms)                                                                      
    [+] DeploymentTemplate Must Not Contain Hardcoded Uri (7 ms)                                                            
    [+] DeploymentTemplate Schema Is Correct (6 ms)                                                                         
    [+] Dynamic Variable References Should Not Use Concat (6 ms)                                                           
    [+] IDs Should Be Derived From ResourceIDs (12 ms)                                                                      
    [+] Location Should Not Be Hardcoded (7 ms)                                                                             
    [+] ManagedIdentityExtension must not be used (5 ms)                                                                    
    [+] Min And Max Value Are Numbers (6 ms)                                                                                
    [+] Outputs Must Not Contain Secrets (7 ms)                                                                            
    [+] Parameters Must Be Referenced (19 ms)                                                                               
    [+] providers apiVersions Is Not Permitted (8 ms)                                                                       
    [+] ResourceIds should not contain (7 ms)
    [+] Resources Should Have Location (7 ms)
    [+] Resources Should Not Be Ambiguous (6 ms)
    [+] Secure Params In Nested Deployments (12 ms)
    [+] Secure String Parameters Cannot Have Default (7 ms)
    [+] Template Should Not Contain Blanks (9 ms)
    [+] Variables Must Be Referenced (16 ms)
    [+] Virtual Machines Should Not Be Preview (27 ms)
    [+] VM Images Should Use Latest Version (7 ms)
    [+] VM Size Should Be A Parameter (9 ms)
Validating templates\azuredeploy.parameters.json
  deploymentParameters
    [+] DeploymentParameters Should Have ContentVersion (9 ms)
    [+] DeploymentParameters Should Have Parameters (8 ms)
    [+] DeploymentParameters Should Have Schema (10 ms)
    [+] DeploymentParameters Should Have Value (6 ms)
Total : 32
Fail  : 0
Pass  : 32

Trying to skip the JSONFiles Should Be Valid


C:\arm-ttk\Test-AzTemplate -TemplatePath C:\Users\gjspoelstra\Repos\Catalog\Catalog\Keyvault\templates\azuredeploy.json  -verbose -skip 'JSONFiles Should Be Valid'                           

VERBOSE: Conversion from JSON failed with error: After parsing a value an unexpected character was encountered: o. Path 'variables.kvtAccessPoliciesArray', line 118, position 97.                                                                                                                                                                                      Validating templates\azuredeploy.json                                                                                     
AllFiles                                                                                                                  
    [+] JSONFiles Should Be Valid (22 ms)                                                                                 
deploymentTemplate                                                                                                        
    [+] adminUsername Should Not Be A Literal (14 ms)
    [+] apiVersions Should Be Recent In Reference Functions (8 ms)
    [+] apiVersions Should Be Recent (12 ms)
    [+] artifacts parameter (6 ms)
    [+] CommandToExecute Must Use ProtectedSettings For Secrets (25 ms)
    [+] DependsOn Best Practices (16 ms)
    [+] Deployment Resources Must Not Be Debug (36 ms)
    [+] DeploymentTemplate Must Not Contain Hardcoded Uri (7 ms)
    [+] DeploymentTemplate Schema Is Correct (7 ms)
    [+] Dynamic Variable References Should Not Use Concat (9 ms)
    [+] IDs Should Be Derived From ResourceIDs (13 ms)
    [+] Location Should Not Be Hardcoded (10 ms)
    [+] ManagedIdentityExtension must not be used (6 ms)
    [+] Min And Max Value Are Numbers (8 ms)
    [+] Outputs Must Not Contain Secrets (7 ms)
    [+] Parameters Must Be Referenced (22 ms)
    [+] providers apiVersions Is Not Permitted (9 ms)
    [+] ResourceIds should not contain (10 ms)
    [+] Resources Should Have Location (9 ms)
    [+] Resources Should Not Be Ambiguous (7 ms)
    [+] Secure Params In Nested Deployments (13 ms)
    [+] Secure String Parameters Cannot Have Default (7 ms)
    [+] Template Should Not Contain Blanks (7 ms)
    [+] Variables Must Be Referenced (10 ms)
    [+] Virtual Machines Should Not Be Preview (25 ms)
    [+] VM Images Should Use Latest Version (6 ms)
    [+] VM Size Should Be A Parameter (9 ms)
Validating templates\azuredeploy.parameters.json
  deploymentParameters
    [+] DeploymentParameters Should Have ContentVersion (6 ms)
    [+] DeploymentParameters Should Have Parameters (8 ms)
    [+] DeploymentParameters Should Have Schema (8 ms)
    [+] DeploymentParameters Should Have Value (6 ms)
Total : 32
Fail  : 0
Pass  : 32

@spoelly
Copy link
Contributor Author

spoelly commented Aug 5, 2021

@StartAutomating we have tested the change you mentioned and this not seems to fix this issue.
#412

@StartAutomating
Copy link
Collaborator

We have a fix ready and are adding tests to avoid regressions.

@StartAutomating
Copy link
Collaborator

@StephenMolloy we have updated the PR with some additional fixes, please retry and let us know.

@spoelly
Copy link
Contributor Author

spoelly commented Aug 10, 2021

Sorry @StartAutomating it still does not fix this issue :( We tested your branch and see no difference in the result run locally and with the superlinter.

@bmoore-msft bmoore-msft added bug Something isn't working and removed Needs: triage 🔍 labels Aug 18, 2021
@bmoore-msft
Copy link
Contributor

I'm not able to repro after #488 in https://github.com/Azure/arm-ttk/releases/tag/20210818

@spoelly
Copy link
Contributor Author

spoelly commented Aug 20, 2021

@bmoore-msft did you run it with the verbose option?
The issue is still there.


C:\Users\gjspoelstra\Downloads\arm-template-toolkit\arm-ttk> Test-AzTemplate -TemplatePath C:\Users\gjspoelstra\Repos\Catalog\Catalog\Keyvault\templates\azuredeploy.json -verbose                                                              

VERBOSE: Conversion from JSON failed with error: After parsing a value an unexpected character was encountered: o. Path 'variables.kvtAccessPoliciesArray', line 118, position 97.                                                                                                                                                                                      

Validating templates\azuredeploy.json                                                                                     
   AllFiles                                                                                                                  
      [+] JSONFiles Should Be Valid (7 ms)                                                                                  
   deploymentTemplate                                                                                                        
      [+] adminUsername Should Not Be A Literal (10 ms)                                                                       
      [+] apiVersions Should Be Recent In Reference Functions (8 ms)                                                          
      [+] apiVersions Should Be Recent (10 ms)                                                                                
      [+] artifacts parameter (5 ms)                                                                                          
      [+] CommandToExecute Must Use ProtectedSettings For Secrets (21 ms)                                                     
      [+] DependsOn Best Practices (8 ms)                                                                                     
      [+] Deployment Resources Must Not Be Debug (9 ms)                                                                       
      [+] DeploymentTemplate Must Not Contain Hardcoded Uri (6 ms)                                                            
      [+] DeploymentTemplate Schema Is Correct (5 ms)                                                                         
      [+] Dynamic Variable References Should Not Use Concat (5 ms)                                                            
      [+] IDs Should Be Derived From ResourceIDs (10 ms)                                                                      
      [+] Location Should Not Be Hardcoded (6 ms)                                                                             
      [+] ManagedIdentityExtension must not be used (5 ms)                                                                    
      [+] Min And Max Value Are Numbers (7 ms)                                                                                
      [+] Outputs Must Not Contain Secrets (6 ms)                                                                            
      [+] Parameters Must Be Referenced (19 ms)                                                                               
      [+] providers apiVersions Is Not Permitted (8 ms)                                                                       
      [+] ResourceIds should not contain (6 ms)
      [+] Resources Should Have Location (5 ms)
      [+] Resources Should Not Be Ambiguous (6 ms)
      [+] Secure Params In Nested Deployments (8 ms)
      [+] Secure String Parameters Cannot Have Default (7 ms)
      [+] Template Should Not Contain Blanks (6 ms)
      [+] Variables Must Be Referenced (10 ms)
      [+] Virtual Machines Should Not Be Preview (21 ms)
      [+] VM Images Should Use Latest Version (7 ms)
      [+] VM Size Should Be A Parameter (7 ms)
Validating templates\azuredeploy.parameters.json
   deploymentParameters
      [+] DeploymentParameters Should Have ContentVersion (5 ms)
      [+] DeploymentParameters Should Have Parameters (5 ms)
      [+] DeploymentParameters Should Have Schema (5 ms)
      [+] DeploymentParameters Should Have Value (6 ms)
Total : 32
Pass  : 32
Fail  : 0

@bmoore-msft
Copy link
Contributor

I did not use the -verbose option... thanks for clarifying (again ;))

@spoelly
Copy link
Contributor Author

spoelly commented Aug 23, 2021

No problem 😉 thanks for reopening the issue

@bmoore-msft
Copy link
Contributor

this is related to variable expansion which is not in use yet... could change it to write-debug short term but need to make sure we get to root cause before consuming var expansion

@bmoore-msft
Copy link
Contributor

we changed this to a write-debug instead (that's really what that case was meant for) - hopefully that doesn't make super linter think failure... (in 2021.08.23

@spoelly
Copy link
Contributor Author

spoelly commented Aug 24, 2021

@bmoore-msft it is still letting the super linter fail :(

@bmoore-msft
Copy link
Contributor

Do you know which version the super linter is using? (or do you see DEBUG: instead of VERBOSE:?) If it's failing on write-debug (which seems as odd as failing on write-verbose so certainly possible) then we'll need another option (obviously)

@StartAutomating fyi

@spoelly
Copy link
Contributor Author

spoelly commented Aug 25, 2021

The super linter always uses the latest arm-ttk release from GitHub.

@StartAutomating
Copy link
Collaborator

This seems like we might want to open an issue on SuperLinter to track this.

IMHO/educated guess: the text saying "error" inside a verbose/debug is causing a failure, and should not be.

That stated, we also want to determine why your (or anyone's) template's variable expansion does not work as expected: This additional logging helps this end goal.

@bmoore-msft bmoore-msft added this to the M1 milestone Sep 10, 2021
@spoelly
Copy link
Contributor Author

spoelly commented Sep 30, 2021

Is there any update on this?

@StartAutomating
Copy link
Collaborator

StartAutomating commented Nov 5, 2021

@spoelly - I'm not seeing this reproduce with what is currently in master (including with -Verbose (tried on PowerShell core and Windows PowerShell). Can you verify it is ongoing?

(updated: It looks like we moved those messages to the -Debug channel).

StartAutomating added a commit to StartAutomating/arm-ttk that referenced this issue Nov 5, 2021
@StartAutomating
Copy link
Collaborator

Upon further investigation, it appears we were not escaping double quotes correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants