Allow all CxSCA configuration options in application.yml + Config as Code combo

[brenwhyte]

Description

I'm trying to define CxSCA properties in application.yml along with developer supplied CxSCA properties in cx.config Seems CxSCA properties in application.yml are ignored or not supported. (not supported I imagine)

Expected Behavior

Right now I'd like for developers to "opt-in" to CxSCA usage. My application.yml looks like:

*snip*
  enabled-vulnerability-scanners:
    - sast
#    - sca

...

sca:
  appUrl: https://sca.checkmarx.net
  apiUrl: https://api-sca.checkmarx.net
  accessControlUrl: https://platform.checkmarx.net
  tenant: 
  username: 
  password: 
  filter-severity:
    - high
#    - medium
#    - low

*snip

And opting-in a dev just needs to add the below to their cx.config:

{   "team": "/CxServer/Team/Squad",
    "additionalProperties": {
        "cxFlow": {
            "vulnerabilityScanners": ["sca", "sast"]
        }
}

That works are expected, the SCA properties for app, api, access control urls are used but all CxSCA scans end up with a Teamname of "All users"

Enter issue #634 That now allowed for Devs to set their Team name via Config as Code, brilliant. eg:

{
    "team": "/CxServer/Team/Squad",
    "additionalProperties": {
        "cxFlow": {
            "vulnerabilityScanners": ["sca", "sast"]
        }
    },
    "sca": {
        "team": "/CxServer/Team/Squad"
    }
}

Actual Behavior

It looks like the CxSCA settings above defined in application.yml (and also the Environment Variables like SCA_TENANT) are being ignored. So if a user tries the above, we'll see this logged in the CxFlow logs:

2021-03-19 12:54:09.599 ERROR 1 --- [      flow-web1] c.checkmarx.flow.config.FlowAsyncConfig  : Error has occurred calling initiateAutomation. MachinaRuntimeException: sca scan failed.
Scan cannot be initiated. SCA application URL wasn't provided

The only way this now works is if developers specify all the CxConfig settings, eg

    "sca": {
        "appUrl": "https://sca.checkmarx.net",
        "apiUrl": "https://api-sca.checkmarx.net",
        "accessControlUrl": "https://platform.checkmarx.net",
        "tenant": "my-tenent",
        "team": "/CxServer/Team/Squad"
    }

Ideally I'd like to manage that for folks so all they need to worry about is say Team name, filter settings etc.

Environment Details

GitLab SaaS CxFlow 1.6.19

[AvivCx]

Hi @brenwhyte From your description it sounds like a bug. we will look into it @NimrodGolan

Thanks, Aviv

[jbrotsos]

This is expected behavior. We use the sca object defined in the config-as-code file. Please continue to use in your config-as-code file:

"sca": {
    "appUrl": "https://sca.checkmarx.net",
    "apiUrl": "https://api-sca.checkmarx.net",
    "accessControlUrl": "https://platform.checkmarx.net",
    "tenant": "my-tenent",
    "team": "/CxServer/Team/Squad"
}

[conormancone-cimpress]

Hi @jbrotsos,

The problem is that that causes more problems than it solves, and violates the principle of least surprise. If the SCA block is all-or-nothing, then a team that wants to redefine just one property must redefine everything. This obviously makes a lot more "noise" - if I want to specify my team I also have to specify everything else. It also causes long-term problems though. Developers are going to hard-code tenant/url/access control/etc details simply so they can specify an alternate team. If later on we end up having to change tenant/url/access control/etc, then it can no longer happen centrally - every team will have to update their cx.config, even if they never cared about changing those details in the first place.

Besides, this isn't necessary elsewhere. The fact that I don't have to re-define every aspect of the application.yml file when I first create the original cx.config file already shows that the expectation is that I only need to define the pieces that I want to change, with everything else just inheriting from the application.yml file. So why is it different for the SCA "block"? That will surprise a lot of people, just like it surprised me.

Looks like a bug from here.

[jbrotsos]

@conormancone-cimpress The reason it's marked as an enhancement is because that is the way it was designed and since it works as expected, I marked this as a lower priority.

But I definitely understand and agree on your point and we will address this accordingly.