feat: Update validation to use validate against latest schema#733
feat: Update validation to use validate against latest schema#733sshiells-scottlogic wants to merge 10 commits intofinos:mainfrom
Conversation
✅ Deploy Preview for common-cloud-controls canceled.
|
sshiells-scottlogic
changed the title
There was a problem hiding this comment.
@eddie-knight just an FYI - this started to flag validation issues in the UI for in the common files (which have a different file structure).
|
@eddie-knight I think this should have the inline comment identifiers back in place. If you spot any that are missing then there is something wrong with the new validation code. There are some errors left, somehow ai-ml/mlde has controls for mlde threats, but no threats in the file?
|
|
Yeah, that's a long-standing error — it seems we merged in the controls without the accompanying threats a while back. See the files in the commit prior to this merge: |
|
So how do we fix this? For me it doesn't really make sense (in the context of the project) to have controls for threats that don't exist? @mlysaght2017 any thoughts? |
services/crypto/key/controls.yaml
Outdated
| threat-mappings: | ||
| - reference-id: CCC | ||
| identifiers: | ||
| - CCC.KeyMgmt.TH01 # Deletion or disabling of KMS key versions leading to denial of service or data destruction |
There was a problem hiding this comment.
@eddie-knight does this look like a reasonable way to split long comments to pass linting? I have used the '|' as a separator so the validator knows to check the next line.
There was a problem hiding this comment.
Reasonable, absolutely.
Though — if we can manage it — I think I'd prefer to enforce a max character length for titles
There was a problem hiding this comment.
What length would you like and can you suggest an alternative title for this one and I'll try to make it happen.
I'd likely do that as a separate PR just to keep things cleaner.
There was a problem hiding this comment.
I don't recall the error this was trying to solve, but I suppose it'd be something beneath that
There was a problem hiding this comment.
It seems to be as simple as
So we just need to decide where we need limits and what we want them to be.
Just an FYI we discussed this on the call and we decided to flag these as warnings as they are not a priority to fix so I have adjusted the validatior to handle this case. |
|
This issue will be closed as stale in 7 days. If this issue is blocked, |
|
Closed as stale. An update may reopen this PR. |
2 participants
No description provided.