Add support for artifact *//*//

[jwikman]

AL-Go version

7.3

Describe the issue

We recently migrated our AppSource app repositories from a custom solution on DevOps to GitHub and AL-Go.
Now we are in the process of preparing all apps for the next major (BC27).

On DevOps we ran our pipelines on the latest current version, if the application property in app.json was lower or equal to current version. When application property was higher than current version, we switched to nextminor/nextmajor artifact.

So far on GitHub, we've used the artifact setting ////daily in our OrgSettings, to utilize image caching on our self-hosted runners. This gives us a container on the current version (BC26, as of today).
This worked fine, until now when we set "application": "27.0.0.0". As expected, it won't compile with the daily artifacts.

Now I noticed this neat feature that at first sight seemed to be what I was looking for; a special artifact setting called //*//first. This really looked like something we could use for our AppSource apps, without needing to change the artifact setting back and forth in all repositories.

But (!) it seems this did not work together with nextmajor, as I'd expect it to. The Determine ArtifactUrl job fails with this:

Error: Unexpected error when running action. Error Message: No artifacts found for the artifact setting (//*//first) in .AL-Go\settings.json, when application dependency is 27.0.0.0, StackTrace: at DetermineArtifactUrl, E:\actions-runner-01\_work\_actions\microsoft\AL-Go-Actions\v7.3\AL-Go-Helper.ps1: line 1917 <- at <ScriptBlock>, E:\actions-runner-01\_work\_actions\microsoft\AL-Go-Actions\v7.3\DetermineArtifactUrl\DetermineArtifactUrl.ps1: line 14 <- at <ScriptBlock>, E:\actions-runner-01\_work\_temp\c68e7be4-de3b-441a-a6a8-c0b8fa797a52.ps1: line 3 <- at <ScriptBlock>, E:\actions-runner-01\_work\_actions\microsoft\AL-Go-Actions\v7.3\Invoke-AlGoAction.ps1: line 21 <- at <ScriptBlock>, E:\actions-runner-01\_work\_temp\c68e7be4-de3b-441a-a6a8-c0b8fa797a52.ps1: line 2 <- at <ScriptBlock>, <No file>: line 1

Wouldn't it be reasonable to expect that the DetermineArtifactUrl() function checked nextminor/nextmajor if it does not find any ArtifactUrl with the regular filter?

Like this:

            Write-Host "Found $($allArtifactUrls.Count) artifacts for version $version matching application dependency $($projectSettings.applicationDependency), selecting $select."
            if (-not $artifactUrl) {
                Write-Host "No artifacts found for matching application dependency $($projectSettings.applicationDependency), checking nextminor version"
                $artifactUrl = @(Get-BCArtifactUrl -type $artifactType -country $country -select NextMinor -accept_insiderEula | Where-Object { [Version]$_.Split('/')[4] -ge [Version]$projectSettings.applicationDependency })[0]
                if (-not $artifactUrl) {
                    Write-Host "No artifacts found for matching application dependency $($projectSettings.applicationDependency), checking nextmajor version"
                    $artifactUrl = @(Get-BCArtifactUrl -type $artifactType -country $country -select NextMajor -accept_insiderEula | Where-Object { [Version]$_.Split('/')[4] -ge [Version]$projectSettings.applicationDependency })[0]
                    if (-not $artifactUrl) {
                        throw "No artifacts found for the artifact setting ($artifact) in $ALGoSettingsFile, when application dependency is $($projectSettings.applicationDependency)"
                    }
                }
            }

We might also need to wrap the checks in a condition on $artifactType -eq "Sandbox", since this won't work for OnPrem. (It throws an error as well, but would be more informative for OnPrem users)

Expected behavior

If application is higher than the latest currently released version, we should check if we can find a nextminor or nextmajor artifact that satisfies the dependency condition.

Eg. if we have today "application" = "27.0.0.0", we should get a nextmajor URL.

Steps to reproduce

Use //*//first as artifact in AL-Go settings.
Set "application" = "27.0.0.0" in app.json.
Run any workflow.

Additional context (logs, screenshots, etc.)

No response

[spetersenms]

Hi @jwikman I think you are getting this issue because 27* builds are still insider builds and not available in the regular storage account. Can you please try updating your setting to bcinsider//*//first and try again?

[jwikman]

@spetersenms, I'm pretty sure that bcinsider//*//first is not allowed. If I recall, when version is *, only select part of the string can be set. (I'm away from computer until late Sunday, so cannot check...)

I know how I could set my artifact to get nextmajor, but I'm used to have a artifact setting that can be used all year around, and we only update app.json as needed. If I change to nextmajor in reposettings now, I must revert that change in the beginning of October. This has to be managed for every repository. That feels unnecessary since we have all data at hand to figure out the expected artifactUrl, don't you agree? 🙂

[jwikman]

@spetersenms, I was wrong earlier.
"bcinsider" is allowed for the storageAccount part of the artifact setting. But it still do not work to have one single setting that works around the lifecycle of a year.

When using "bcinsider", only pre-release version can be found.

artifact = 'bcinsider//*//first'
applicationDependency = '26.0'
Found 0 artifacts for version 26.0 matching application dependency 26.0, selecting first.
ERROR: No artifacts found for the artifact setting (bcinsider//*//first) in $ALGoSettingsFile, when application dependency is 26.0

When not specifying a storageAccount, no pre-release version can be found

artifact = '//*//first'
applicationDependency = '27.0'
Found 0 artifacts for version 27.0 matching application dependency 27.0, selecting first.
ERROR: No artifacts found for the artifact setting (//*//first) in $ALGoSettingsFile, when application dependency is 27.0

Isn't the storageAccount setting just an old relic from when you needed to get the secret SAS Token to access them?
I guess that getting rid of that one probably won't happen before removing BcContainerHelper from AL-Go.

But solving this with a nice behavior in AL-Go wouldn't be that hard IMO.

I just need one single artifact that we can use on all our AppSource app repos, and the application property in each app.json would be used for picking the right artifact url.

[spetersenms]

Hi @jwikman. I will bring this up with the team to discuss how to handle this going forward. This is not really a bug as the system works as designed, but as you say it is somewhat outdated. Some sort of fallback option that checks both storage accounts probably makes sense, but we have to prioritize how that work fits into the rest of our plans.
Keeping both storage accounts do still make sense though, as the EULA for bcinsider is different from bcartifacts.

In case you update all your apps to 27.0 at the same time, you could put the artifact setting at the org level so you only have to update it in one place, but of course that won't work if some apps use 26.0 and others 27.0.

[jwikman]

@spetersenms, yeah, I understand.

We actually configured artifact on org level this morning, so that will be the way forward until we get some more streamlined option.

Please do not hesitate to reach out for feedback on future features around this topic. We had a lot of automated options around this in our (now legacy) DevOps solution, so we might have some experience to contribute with. 🙂

Just as FYI on our current setup, we couldn't use bcinsider//*//first for two reasons

1. first available build on next major is quite old by now, we would probably always need the latest nextmajor version
2. the * version checks application in app.json, but we will not be able to lift all apps to dependency on BC27 yet, so we need to set version explicit in the artifact, otherwise the pipelines will fail on those with application < nextmajor. We do not update all apps, if not needed.

Above lead us to set "artifact" : "bcinsider//27.0//latest" (where 27.0 needs to updated for each major) in OrgSettings, sometime around August / February when we start to work on the next major changes required in our apps. Then after the wave is released, we can switch back to ////daily for CICD and //*//latest for our PR pipelines.

[spetersenms]

@jwikman thanks for the update. Your current setup sounds like the best way to do things with the current version of AL-Go.

I think it would be great to have a conversation, and i would be very interested in hearing your story on migrating to AL-Go. Could you please reach out to me on spetersen@microsoft.com which your preferred contact information?

[jwikman]

@spetersenms, you've got mail. 🙂

[jwikman]

I can do a PR for this, if we can decide on how things would work.

I gave this some more thoughts, and came to these conclusions:

• bcinsider//*//first is not useful, since the first available nextmajor is way too old. bcinsider//*//latest is the only usable artifact when running against nextmajor.
• For AppSource apps we would like to use //*//first for apps that do not have application property set to nextmajor (I.e. current or older major)
• If we have application property set to the next minor version (which is plausible after the wave is released), we would like to have a variant of bcinsider//*//latest that returns the latest bcinsider for that minor, and not the nextmajor.

This makes me think that none of the //*//first or //*//latest would work for our purposes. We want an artifact that is fully automated from the application property.

One suggestion would be to introduce a new "special AL-Go" artifact: //*//
This will then have the following behavior:

• If application property <= ////daily - Use the same behavior as for //*//first
• if application property > ////daily:
  • if major of application property = major from ////daily - Use the latest version of nextminor of bcinsider
  • if major of application property > major from ////daily - Use the same behavior as for bcinsider//*//latest

//*// will of course also be documented, together with //*//first and //*//latest.

[jwikman]

Now we've started to work on our AppSource apps and nextmajor, and of course this request advanced on our wishing list! :)

@spetersenms @mazhelez, any thoughts on above? 🙂
Would you accept such a PR?

I believe the only change needed is inside the DetermineArtifactUrl() function in AL-Go-Helper.ps1

[spetersenms]

Hi @jwikman, I converted the issue to a discussion since this is now an idea discussion and not really a bug.

We would be happy to accept a contribution. Regarding your implementation idea in the previous comment, I am not sure using //*// is a good idea, as that is already legal syntax and could potentially break existing users. How about instead using something like *//*// as the special artifact? That would help symbolize that the artifact could come from either the regular or insider storage account.

[jwikman]

Hi @spetersenms,

I couldn't convert this to discussion myself, so I'm glad you did that :)

Your idea with *//*// sounds perfect! 👍

I'll get back with a PR on this as soon I find the time to do this.