-
Notifications
You must be signed in to change notification settings - Fork 125
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
Add support for indirect template + customizations #853
base: main
Are you sure you want to change the base?
Conversation
@freddydk, do you have an estimated time of completion for this PR? We've been testing it for a while and are considering deploying it live. However, we're a bit cautious about when the PR will be finalized and whether it will disrupt any modifications we've implemented. |
@jonaswre, @mortenseifert and @lippertmarkus - feel free to add your code review as well |
Hi @freddydk, i'm currently on vacation until September 2, but I'll test it in my demo org once im back. Im excited for this change. But trusted nuget feeds would be my first priority. We've recently got more requests to use our apps as dependencies so will need to distribute runtimes to customers. We've implemented the GenerateBCNuget Repo which works great but distributing them is the next step for us. If I can help with that let me know. |
I will resume the work on NuGet while this PR is in review. |
@@ -186,6 +186,19 @@ try { | |||
} | |||
$authContext = $null | |||
$environmentName = "" | |||
if ($cicdAuthContext) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a new setting/secret? Do we have docs on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the setting for allowing us to run tests in online sandboxes - and the setting Cosmo needs in order to run pipelines with their Alpaca containes.
Could be moved to a seperate PR - but is needed together with this for Cosmo.
Will add it to the docs.
} | ||
# Merge permissions | ||
Write-host "Merge permissions" | ||
$srcPermissionsObj = $srcYaml.Get('permissions:/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't there be more than on permissions definition? You can also set permissions on jobs
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token#setting-the-github_token-permissions-for-a-specific-job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually thought that the upper permissions setting was the permissions allowed for all jobs and no job could set additional permissions.
When I read the docs now - it looks like the upper permissions settings is indeed the permissions allowed for all jobs, but jobs can add additional permissions if they like (except for the reusable workflows).
Will check whether this changes anything,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posting an mid-way review as I got carried away. I'll circle back to this PR.
|
||
### New Settings | ||
|
||
- `customALGoSystemFiles` is an array of JSON objects, which holds information about custom AL-Go System Files, which will be applied during Update AL-Go System Files. Every object can hold these 4 properties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's time we stop adding everything in the AL-Go settings.
These settings are loaded and output in a lot of jobs unnecessarily.
"Update AL-Go" has two phases:
- Fetch changes from the template. (here
customALGoSystemFiles
is needed) - Modify the YAML workflows based on the settings (here
customALGoSystemFiles
is not needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of agree, but I also don't think we should have other settings files.
IMO we should make the same change to settings as I did to secrets a while back, where we specify which secrets we want to read when calling ReadSecrets.
So, the workflows should know (and specify) which settings to look for and the only settings returned would be the once specified. This way the settings carried around in outputs would be limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user I can say having more setting files would make it more complex. Most of the users I know prefer one big setting file where they have everything in one file.
Co-authored-by: Maria Zhelezova <[email protected]>
Co-authored-by: Maria Zhelezova <[email protected]>
By adding a setting called [`customALGoSystemFiles`](https://aka.ms/algosettings#customalgosystemfiles) in your [repo settings](https://aka.ms/algosettings#settings), you can tell AL-Go for GitHub that these files should be included in the update. Example: | ||
|
||
``` | ||
"customALGoSystemFiles": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is customALGoSystemFiles
supposed to be defined on the indirect template or on the end repositories (the ones that consume the indirect template)?
If it's on the end repos, then it defeats part of the purpose of the indirect template. E.g. in the case of a new common script, the setting needs to be added manually on X repos.
If it's on the indirect template template, then shouldn't relative paths suffice? Does it need to be so complex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean by relative paths?
The Source is an absolute URL to a file to download - rarely something in the same repo.
The setting can be on both.
If it is defined on the indirect template - it is handled when updating the indirect template.
If it is defined on the end repos - it is handled when updating the end repo (not everybody will have an indirect template)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not require an indirect template if you want to sync custom scripts? Is there a requirement for two different mechanisms of syncing scripts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are two different things.
An indirect template is something people create in their organization for types of repos.
custom files could be workflows or scripts created by other people (like the onprem publisher created by a partner)
Co-authored-by: Maria Zhelezova <[email protected]>
Co-authored-by: Maria Zhelezova <[email protected]>
}, | ||
{ | ||
"Destination": ".github/", | ||
"Source": "https://github.com/freddydk/CustomALGoSystemFiles/archive/refs/heads/main.zip", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we authenticate to a source if it is in private repo? Is that supported?
Write-Host "Create Tag" | ||
|
||
PostProcess: | ||
needs: [ Initialization, Build2, Build1, Build, Deploy, Deliver, DeployALDoc, CustomJob-CreateBuildTag ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I did something wrong but this didn't work for me. CustomJob-CreateBuildTag is not added in the "needs" block in my repository when I add it in my indirect template.
This is my indirect template:
PostProcess:
needs: [ Initialization, Build, BuildPP, Deploy, Deliver, CustomJob-CreateBuildTag, DeployALDoc ]
if: (!cancelled())
runs-on: [ windows-latest ]
steps:
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- name: Finalize the workflow
id: PostProcess
uses: aholstrup1/AL-Go/Actions/WorkflowPostProcess@ab27cdbe117b8c1102c0be563f2e098be079ce37
env:
GITHUB_TOKEN: ${{ github.token }}
with:
shell: powershell
telemetryScopeJson: ${{ needs.Initialization.outputs.telemetryScopeJson }}
currentJobContext: ${{ toJson(job) }}
CustomJob-CreateBuildTag:
name: Create Build Tag
needs: [ Initialization, Build ]
if: (!cancelled()) && (needs.Build.result == 'success')
runs-on: [ ubuntu-latest ]
steps:
- name: Create Tag
run: |
Write-Host "Create Tag"
This is from my repository that uses the indirect template (after running Update AL-Go System Files)
PostProcess:
needs: [ Initialization, Build, Deploy, Deliver, DeployALDoc ]
if: (!cancelled())
runs-on: [ windows-latest ]
steps:
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- name: Finalize the workflow
id: PostProcess
uses: aholstrup1/AL-Go/Actions/WorkflowPostProcess@ab27cdbe117b8c1102c0be563f2e098be079ce37
env:
GITHUB_TOKEN: ${{ github.token }}
with:
shell: powershell
telemetryScopeJson: ${{ needs.Initialization.outputs.telemetryScopeJson }}
currentJobContext: ${{ toJson(job) }}
CustomJob-CreateBuildTag:
name: Create Build Tag
needs: [ Initialization, Build ]
if: (!cancelled()) && (needs.Build.result == 'success')
runs-on: [ ubuntu-latest ]
steps:
- name: Create Tag
run: |
Write-Host "Create Tag"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange - when you add that to the indirect template - it shouldn't do anything in the end repo.
Will have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it add CustomJob-CreateBuildTag to the needs block in PostProcess? 🤔 Maybe I misunderstood it
Changes included in this PR:
In this case, org/myptetemplate is a standard AL-Go repository, which gets updated as any other AL-Go repository using Update AL-Go System Files. This is a way of standardizing your repository based on type specific AL-Go templates (which can be private or public).
Normally, Update AL-Go System Files follows this mechanism:
Workflows and scripts are read from the template repository and modified based on settings and saved to the repository.
Using an indirect template repository, the picture looks like this:
Workflows and scripts are read from the "real" template repository and modified based on settings. Customizations for workflows in the indirect template repository are applied.
New workflows and scripts from the indirect template repository are copied over and new settings (for repo and for project) are copied over (if they don't already exist in the corresponding settings file).
The final repository can also contain customizations to workflows and the winner here is the indirect template repository (if a custom job or custom step exists in both places).
This PR: freddydk/customized#17 - is an example of a repository with customizations of CI/CD and _BuildALGoProject, which runs Update AL-Go System Files and gets customizations from the indirect template repository (both settings, scripts and workflow customizations)
TODO:
Documentation on customization capabilities is here: https://github.com/freddydk/AL-Go/blob/customize/Scenarios/CustomizingALGoForGitHub.md