restRoot schema properties check is incorrectly too restrictive

[guilhemmarchand]

Hi,

A recently introduced change mades the UCC gen way too much, and incorrectly, restrictive regarding the Addon naming convention.

Example of failure:

Some functions will not be available outside of a splunk hosted process
2021-04-24 14:54:24,899 [UCC] INFO: Cleaning out directory /Users/guilhem/DocsOffsync/git/splunk/addonfactory-ucc-generator/TA-jira-service-desk-simple-addon/data/output
2021-04-24 14:54:24,899 [UCC] INFO: Cleaned out directory /Users/guilhem/DocsOffsync/git/splunk/addonfactory-ucc-generator/TA-jira-service-desk-simple-addon/data/output
2021-04-24 14:54:24,920 [UCC] ERROR: Config is not valid. Error: 'TA-jira-service-desk-simple-addon' does not match '^\\w+$'

Failed validating 'pattern' in schema['properties']['meta']['properties']['restRoot']:
    {'pattern': '^\\w+$', 'type': 'string'}

On instance['meta']['restRoot']:
    'TA-jira-service-desk-simple-addon'

This change was I believe introduced in the following commit:

0782921

The regular expression should not forbid the usage of hypens, while this is not good practice anymore in Appinspect, this is only a warning not a failure, and the UCC gen must not decide on its own to prohibit its usage.

[ghost]

Hi @guilhemmarchand, I pinged one of the engineers involved in this repository.

[ghost]

Hi @guilhemmarchand

We would like to leave it as it is now.

To fix this issue from your side - you can change "TA-jira-service-desk-simple-addon" to "TA_jira_service_desk_simple_addon".

[guilhemmarchand]

In my opinion this does not make sense @arys-splunk

If we would consider this as something mandatory, Appinspect would raise this as a failure and not a warning, and we would forbid this for instance in Splunk Cloud.

"To fix this issue from your side - you can change "TA-jira-service-desk-simple-addon" to "TA_jira_service_desk_simple_addon"."

This is nothing like an answer, changing an app name space has so many implications from the app itself point of view of course (anything relying on the app name space) to the cost, extrem cost, you would be asking to customers to migrate your app from version x to y just because someone thinks it is smarter to use _ everywhere instead to -

For a customer this means litterarly uninstalling the app, which many will not notice / read / understand so many will simply fail etc and spend hours and hours for nothing - calling the dev to spend hours and hours too. So I am sorry but that's only an easy answer.

Issuing a warning can make sense, forbidding it a mistake and a non sense.

Please re-consider this.

[ghost]

@guilhemmarchand I reopened the issue and your comment makes sense for me, did not think about the potential customer implications. Let me pull @rfaircloth-splunk here and will try to figure this out.

[ghost]

@guilhemmarchand Can you please share what are you trying to achieve? From what you are talking

changing an app name space

do you mean that you want to change the name of the add-on?

I assume that you are talking about https://github.com/guilhemmarchand/TA-jira-service-desk-simple-addon repository. From what I see it has https://github.com/guilhemmarchand/TA-jira-service-desk-simple-addon/blob/master/TA-jira-service-desk-simple-addon/appserver/static/js/build/globalConfig.json#L7 and it has restRoot as "TA_jira_service_desk_simple_addon" which is intended.

From my investigation - this '^\\w+$' regex was always present in the schema for globalConfig.json. It was not just validated before when running ucc-gen command. And proving incorrect restRoot would lead to an error in Splunk UI.
Also I confirmed that AOB (Add-on Builder) created add-on does not use dashes in restRoot.

[guilhemmarchand]

Another addition to this topic, Splunk UI does not allow the usage of - and replaces any hyphen with an underscore...
https://splunkui.splunkeng.com/Create/AppTutorial

[guilhemmarchand]

Hi @arys-splunk !
Sorry for late reply, I didn't see the updates.

All right, I see what you mean, so actually, I was trying to create the app from scratch using the ucc-gen.

The reason why I am trying to use this naming is because the app exists already with this naming, which at that time of the original Addon creation wasn't discouraged.

My end goal is to migrate these originally AoB created addons to use ucc, to be do it well - the easiest is to use the target name then take what I had to.

Thus, it's no big deal as I can deal with it, I opened this issue because it seemed to be to me unjusitified to "refuse" using hyphens, submiting a warning and recommandation why not, but drastically refusing it while many other parts use hypens (as SUI)

To summarise, I mean the root directory name of the Splunk app, what's happening underneath is good to know for sure.

I'll let you choose to re-close the issue or not, my opinion was that prohibiting it in the root directory name of the app when using ucc-gen to generate the Addon is too radical and not justified.