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

WIP: Bug 1001395 Create manifest names in correct format #122

Open
wants to merge 1 commit into
base: sunny/arm-infra-template-support
Choose a base branch
from

Conversation

sunnycarter
Copy link
Collaborator

@sunnycarter sunnycarter commented Oct 31, 2023

Work in progress bugfix for https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/1001395

-Raised by: Graeme Robertson (HE/HIM): How are artifact manifests used? Specifically I'm c...
posted in AFO Technical Team / AOSM Help on 30 October 2023 17:32

Context
Mountain-lake couldn't use nsd publish as normal because the CLI had created a manifest name that is over the 50 character limit.

(AOSM has a wide range of requirements for validation that are not documented in the API. One thing that was supposed to be happening at documentation time was that these were documented. I don't believe they were. See rambly discussion here:
Jacob Darby: Allow list for ARM resources deployed by an NF
posted in A40 LCM Team / UK - General on 27 September 2023 09:59)

The particular problem here are the validation requirements on artifact manifest names:
https://dev.azure.com/msazuredev/AzureForOperators/_git/pez?path=/src/MultiaccessEdgeCompute/ArtifactManifest/Errors/ArtifactManifestValidationErrorMessages.cs&_a=contents&version=GBmain for error messages and https://dev.azure.com/msazuredev/AzureForOperators/_git/pez?path=/src/MultiaccessEdgeCompute/ArtifactManifest/Validators/ArtifactManifestCreationValidator.cs&_a=contents&version=GBmain for strings.

Jacob argues that the CLI should not repeat the Validation that AOSM does, as it is the job of the RP. However the CLI creates a whole load of stuff at once and it's annoying to get half way through and then fail. In this case, it doesn't fail until deploy time.

In the particular case of manifest names, the CLI creates those out of combinations of values from the input.json file, versions and constants, e.g.
ubuntu-vm-nfdg-nf-acr-manifest-1-0-0

With a 50 character limit, you don't need a long nf name to exceed the requirement

Sunny started work on this but had these worries:

  1. Can we change manifest names in the CLI without it being a breaking change? I think it would only break delete functionality, not anything to do with publish - if you tried to delete a NSD/NFD created by an old version of the CLI and it had different names
  2. Should we raise an exception if the manifest name is too long, or truncate it? If truncating, we'd chop the version off the end, which wouldn't be good as having unique manifest name per version is important for publishing new NFDs / NSDs
  3. See linked bug - what do we do about option to pass in --manifest-params-file argument?
  4. What other fixes do we need, e.g. artifact names / artifact + version length for storage backed artifact stores (see pez links above) - we should try to generally do better at not creating things that will fail validation.

@jordlay
Copy link
Collaborator

jordlay commented Mar 11, 2024

We should ensure this validation exists in our new CLI. Keeping PR open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants