Lightning-Flow-Scanner / lightning-flow-scanner-sfdx

A Salesforce CLI Plugin designed to pinpoint deviations from Industry Best Practices in Salesforce Flows, ensuring standards of business automation excellence.
https://www.npmjs.com/package/lightning-flow-scanner
GNU Affero General Public License v3.0
112 stars 11 forks source link

--failon command does not return status 1 #71

Closed SFDX-Sam closed 9 months ago

SFDX-Sam commented 1 year ago

Good evening! I've implemented flow scanner on both a gitlab pipeline and my local windows machine, and I cannot get the scanner to return a status code of 1 when using the --failon command.

I run the following command: sf flow:scan --failon error --config "scanner-config.json"

And it returns:

Identified 1 flows to scan... Scan complete
== DML Inside Loop Flow ==
Flow type: AutoLaunchedFlow

ERROR APIVersion
Details: 56
Newer API components may cause older versions of Flows to start behaving incorrectly due to differences in the underlying 
mechanics. The Api Version has been available as an attribute on the Flow since API v50.0 and it is recommended to limit 
variation and to update them on a regular basis.

ERROR FlowName
Details: The name DML_Inside_Loop_Flow does not meet the regex convention ===58
Readability of a flow is very important. Setting a naming convention for the Flow Name will improve the 
findability/searchability and overall consistency. It is recommended to at least provide a domain and a short description of 
the actions undertaken in the flow, in example Service_OrderFulfillment.

=== A total of 2 errors have been found in 1 flows.`

But does not seem to throw a status code of 1 - I have tried reducing the loglevel down to warning but still no luck.

image

SFDX-Sam commented 1 year ago

For reference - my gitlab pipeline looks like this:

####################################################
# The docker image the jobs initialize with.
# We use nodejs, so a node image makes sense.
# https://docs.gitlab.com/ee/ci/yaml/README.html#image
####################################################
image: "node:latest"

####################################################
# The sequential stages of this pipeline.
# Jobs within a stage run in parallel.
# https://docs.gitlab.com/ee/ci/yaml/README.html#stages
####################################################
stages:
  - metadata_scan
  - apex_scan

####################################################
# Metadata Scanning Phase
####################################################
metadata_scan:
  stage: metadata_scan
  allow_failure: false
  script:
    - install_salesforce_cli
    - install_lightning_flow_scanner
    - scan_metadata

  artifacts:
    paths:
      - metadata-scan-files/
    when: always

####################################################
# Helper Methods
####################################################

.sf_helpers: &sf_helpers |
  function install_salesforce_cli() {

    # Salesforce CLI Environment Variables
    # https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_cli_env_variables.htm

    # By default, the CLI periodically checks for and installs updates.
    # Disable (false) this auto-update check to improve performance of CLI commands.
    export SF_AUTOUPDATE_DISABLE=false

    # Set to true if you want to use the generic UNIX keychain instead of the Linux libsecret library or macOS keychain.
    # Specify this variable when using the CLI with ssh or "headless" in a CI environment.
    export SF_USE_GENERIC_UNIX_KEYCHAIN=true

    # For force:package:version:create, disables automatic updates to the sfdx-project.json file.
    export SF_PROJECT_AUTOUPDATE_DISABLE_FOR_PACKAGE_VERSION_CREATE=true

    # Install Salesforce CLI
    npm install @salesforce/cli --global

    # Output CLI version and plug-in information
    sf update
    sf --version
    sf plugins --core

  }
  #Install the flow scanner plugin - it is not digitally signed so we need to echo y to accept the prompt.
  function install_lightning_flow_scanner() {
    echo 'y' | sf plugins:install lightning-flow-scanner
  }

  # Function to run the metadata scan on the current branch
  function scan_metadata() {
    local OUTPUT_DIRECTORY=metadata-scan-files
    mkdir -p $OUTPUT_DIRECTORY
    sf flow:scan --failon error --config "scanner-config.json" 

  }
RubenHalman commented 1 year ago

thank you for reporting this. First of all, I think it would be -failon "error" instead of -failon error.(not sure if that makes a difference) I am wondering however, if you don't use failing it is error level by default as per my understanding. so in this case you don't need the argument, although it should work ofcoùrse. what are you trying to achieve with the failon argument specifically?

SFDX-Sam commented 1 year ago

Hey @RubenHalman !

I think I just put it because the documentation didn't specify if it was required for throwing a non-zero status code or not.

It seems that even without the --failon flag, it still returns a status code of 1 (success).

I also tried -failon "error", but no dice, it did not give me any errors however, so I guess it is an accepted param.

RubenHalman commented 1 year ago

I am sorry. Can you show the --JSON output?

SFDX-Sam commented 1 year ago

Sure thing

{
  "status": 0,
  "result": {
    "summary": {
      "flowsNumber": 1,
      "errors": 2,
      "message": "A total of 2 errors have been found in 1 flows.",
      "errorLevelsDetails": {
        "error": 2
      }
    },
    "status": 1,
    "results": [
      {
        "flowName": "DML Inside Loop Flow",
        "ruleName": "APIVersion",
        "description": "Newer API components may cause older versions of Flows to start behaving incorrectly due to differences in the underlying mechanics. The Api Version has been available as an attribute on the Flow since API v50.0 and it is recommended to limit variation and to update them on a regular basis.",
        "severity": "error",
        "type": "flow",
        "details": "56"
      },
      {
        "flowName": "DML Inside Loop Flow",
        "ruleName": "FlowName",
        "description": "Readability of a flow is very important. Setting a naming convention for the Flow Name will improve the findability/searchability and overall consistency. It is recommended to at least provide a domain and a short description of the actions undertaken in the flow, in example Service_OrderFulfillment.",
        "severity": "error",
        "type": "flow",
        "details": "The name DML_Inside_Loop_Flow does not meet the regex convention ===58"
      }
    ]
  }
}

Interestingly, I can see the status here for each error, but it does not seem to exit the oclif process with the exit code - it seems possible based on a quick look here

RubenHalman commented 1 year ago

I probably need some input from @nvuillam here. Before v2, the core module threw an error resulting in an exit code, but it malformed the json. So I believe we changed it so the json is valid so it can be used for automation, but this is not really my expertise and requires a further deep dive

nvuillam commented 1 year ago

@RubenHalman @SFDX-Sam sorry for the delay (vacations ^^), this issue should be solved in https://github.com/Force-Config-Control/lightning-flow-scanner-sfdx/pull/72 :)

SFDX-Sam commented 1 year ago

Thanks both!

RubenHalman commented 1 year ago

Hi @SFDX-Sam . Sorry for the delay and thank you again for your feedback. @nvuillam fix is included in the latest version 2.12. Please let us know if this sufficiently addresses your needs!

jorgesolebur commented 1 year ago

Hello ! I am still having the same issue, either calling sfdx flow scan -c .flow-scanner.json -d "./force-app/" --json --failon "error" OR sfdx flow scan -c .flow-scanner.json -d "./force-app/" --json --failon error

I am also using the last version

sf plugins
lightning-flow-scanner 2.12.0

This is the response (it includes errors):

{
  "status": 0,
  "result": {
    "summary": {
      "flowsNumber": 27,
      "errors": 5,
      "message": "A total of 5 errors have been found in 27 flows.",
      "errorLevelsDetails": {
        "error": 172,
        "warning": 54
      }
    },
    "status": 1,
    "results": [
      {
        "flowName": "NCS Clone Action Plan Template",
        "ruleName": "MissingFaultPath",
        "description": "Sometimes a flow doesn’t perform an operation that you configured it to do. By default, the flow shows an error message to the user and emails the admin who created the flow. However, you can control that behavior.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "Get_Action_Plan_Template_Details",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Clone Action Plan Template",
        "ruleName": "MissingFaultPath",
        "description": "Sometimes a flow doesn’t perform an operation that you configured it to do. By default, the flow shows an error message to the user and emails the admin who created the flow. However, you can control that behavior.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "GET_Action_Plan_Template_Task_Details",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Opportunity After Save",
        "ruleName": "FlowDescription",
        "description": "Descriptions are useful for documentation purposes. It is recommended to provide information about where it is used and what it will do.",
        "severity": "error",
        "type": "flow",
        "details": "undefined"
      },
      {
        "flowName": "NCS Opportunity Before Save",
        "ruleName": "MissingNullHandler",
        "description": "If a Get Records operation does not find any data it will return null. Use a decision element on the operation result variable to validate that the result is not null.",
        "severity": "error",
        "type": "pattern",
        "details": {
          "name": "Get_Deal_Path_TCV_Matrix_Record",
          "type": "recordLookups"
        }
      },
      {
        "flowName": "NCS Opportunity Before Save",
        "ruleName": "FlowDescription",
        "description": "Descriptions are useful for documentation purposes. It is recommended to provide information about where it is used and what it will do.",
        "severity": "error",
        "type": "flow",
        "details": "undefined"
      }
    ]
  }
}
nvuillam commented 1 year ago

Hmmm it was supposed to be solved :( I'll look into it soon !

jorgesolebur commented 11 months ago

Thanks

jorgesolebur commented 11 months ago

No pressure but do you have any idea on when this will be solved ? We are forced to stop using this fantastic tool in our CI because of this error

RubenHalman commented 11 months ago

@jorgesolebur Im sorry. I have don't understand how this parameter is supposed to be working. Im waiting for@nvuillam or someone to explain this to me so I can implement a fix!

jorgesolebur commented 11 months ago

@nvuillam Please do not hesitate to details on how this works! From my understand, this issue is somehow related to https://github.com/Force-Config-Control/lightning-flow-scanner-sfdx/issues/65, because if there is at least an error in the JSON output summary then the command should fail (returning error code 1). If the JSON output summary called "errors" = 0, then the command should return error code 0.

nvuillam commented 10 months ago

I'll try to fix that this weekend:)

RubenHalman commented 10 months ago

@nvuillam Thanks. I am confused how the error level should impact the exit status.

nvuillam commented 10 months ago

I think I found a fix :)

RubenHalman commented 10 months ago

My apologies for the delay on publishing these changes. It should now be resolved with the latest version thanks to Nicolas efforts. Thank you very much for your involvement @SFDX-Sam, @jorgesolebur @nvuillam

jorgesolebur commented 10 months ago

Thanks! Let me test that! I will be very happy to be able to enable again this automation in my pipeline :-) will keep you posted!

nvuillam commented 10 months ago

@jorgesolebur I integrated Flow Scanner within MegaLinter (it is still in the beta version) , and it requires exit code > 0 in case of errors, so it should work on your side too :)

jorgesolebur commented 10 months ago

@nvuillam Maybe I am missing something, but I am trying to execute the following command: sf flow scan -c .flow-scanner.json -d "./force-app-delta/" --json --failon error

Which would mean that I only want the above command to return status 1 if they find any violation defined with a severity 'error'.

I have defined in my config json file all rules with a severity warning:

"rules":   {
    "APIVersion": {
      "severity": "warning",
      "expression": "===59"
    },
    "CopyAPIName": {
      "severity": "warning"
    },
    "DMLStatementInLoop": {
      "severity": "warning"
    },
    "DuplicateDMLOperation": {
      "severity": "warning"
    },
    "FlowDescription": {
      "severity": "warning"
    },
    "HardcodedId": {
      "severity": "warning"
    },
    "MissingFaultPath": {
      "severity": "warning"
    },
    "MissingNullHandler": {
      "severity": "warning"
    },
    "UnconnectedElement": {
      "severity": "warning"
    },
    "UnusedVariable": {
      "severity": "warning"
    }
  }

When executing the above command, it is returning a status code 1. This is not consistent because the scan found 2 violations that had a severity warning. The scan did not find any violation with severity = error:

{
  "status": 1,
  "result": {
    "summary": {
      "flowsNumber": 1,
      "results": 2,
      "message": "A total of 2 results have been found in 1 flows.",
      "errorLevelsDetails": {}
    },
    "status": 1,
    "results": [
      {
        "violation": {
          "metaType": "attribute",
          "name": "58",
          "subtype": "apiVersion",
          "expression": "===59"
        },
        "name": "58",
        "metaType": "attribute",
        "type": "apiVersion",
        "details": {
          "expression": "===59"
        },
        "ruleDescription": "Introducing newer API components may lead to unexpected issues with older versions of Flows, as they might not align with the underlying mechanics. Starting from API version 50.0, the 'Api Version' attribute has been readily available on the Flow Object. To ensure smooth operation and reduce discrepancies between API versions, it is strongly advised to regularly update and maintain them.",
        "rule": "Outdated API Version",
        "flowName": "NCS Approval Assignment Matrix Header Validation Before Save",
        "flowType": "AutoLaunchedFlow",
        "severity": "warning"
      },
      {
        "violation": {
          "element": {
            "name": [
              "Get_Existing_Approval_Assignment_Matrix_Header"
            ],
            "label": [
              "Get Existing Approval Assignment Matrix Header"
            ],
            "locationX": [
              "182"
            ],
            "locationY": [
              "395"
            ],
            "assignNullValuesIfNoRecordsFound": [
              "false"
            ],
            "connector": [
              {
                "targetReference": [
                  "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
                ]
              }
            ],
            "filterLogic": [
              "1 AND 2 AND 3 AND ((4 AND 5) OR (6 AND 7) OR (9 AND 10)) AND 8"
            ],
            "filters": [
              {
                "field": [
                  "NCS_Country__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Country__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Territory__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Territory__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_CSU__c"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_CSU__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "GreaterThan"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "GreaterThan"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "RecordTypeId"
                ],
                "operator": [
                  "EqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.RecordTypeId"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_Start_Date__c"
                ],
                "operator": [
                  "LessThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_Start_Date__c"
                    ]
                  }
                ]
              },
              {
                "field": [
                  "NCS_End_Date__c"
                ],
                "operator": [
                  "GreaterThanOrEqualTo"
                ],
                "value": [
                  {
                    "elementReference": [
                      "$Record.NCS_End_Date__c"
                    ]
                  }
                ]
              }
            ],
            "getFirstRecordOnly": [
              "true"
            ],
            "object": [
              "NCS_Approval_Assignment_Matrix_Header__c"
            ],
            "storeOutputAutomatically": [
              "true"
            ]
          },
          "subtype": "recordLookups",
          "metaType": "node",
          "connectors": [
            {
              "element": [
                {
                  "targetReference": [
                    "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
                  ]
                }
              ],
              "processed": false,
              "type": "connector",
              "reference": "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
            }
          ],
          "name": "Get_Existing_Approval_Assignment_Matrix_Header",
          "locationX": "182",
          "locationY": "395"
        },
        "name": "Get_Existing_Approval_Assignment_Matrix_Header",
        "metaType": "node",
        "type": "recordLookups",
        "details": {
          "locationX": "182",
          "locationY": "395",
          "connectsTo": [
            "Get_Existing_Approval_Assignment_Matrix_Header_Null_Check"
          ]
        },
        "ruleDescription": "At times, a flow may fail to execute a configured operation as intended. By default, the flow displays an error message to the user and notifies the admin who created the flow via email. However, you can customize this behavior by incorporating a Fault Path.",
        "rule": "Missing Fault Path",
        "flowName": "NCS Approval Assignment Matrix Header Validation Before Save",
        "flowType": "AutoLaunchedFlow",
        "severity": "warning"
      }
    ]
  }
}

Does it make sense ? Shall I crate a new issue ?

RubenHalman commented 9 months ago

@jorgesolebur Thank you very much for your input and im sorry for my limited involvement in this matter. I believe I have now resolved this issue with 2.16, but if you would be so kind to have a look I would greatly appreciate it.

jorgesolebur commented 9 months ago

Thanks! Will test it out soon

jorgesolebur commented 9 months ago

It is working now, thanks ! However I found another issue and will be created a ticket separately.

RubenHalman commented 9 months ago

@jorgesolebur thank you so much for your patience and involvement. we will have a look at that ticket ASAP.