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

Token Replacement: Using naming prefixes to support forking and instantiating the repo for CARML consumers #909

Closed
ahmadabdalla opened this issue Jan 20, 2022 · 7 comments · Fixed by #945
Assignees
Labels
enhancement New feature or request [prio] high importance of the issue: high priority
Milestone

Comments

@ahmadabdalla
Copy link
Contributor

Description

Currently a lot of the resources in the CARML repo use static names for resources (i.e. dependency pipeline resources and resources that must have unique name across Azure like Storage and key vault). We need to brainstorm how we can leverage the existing token replacement service to create name prefixes (i.e. using local tokens in the settings file or another method). And then implement it.

@ahmadabdalla ahmadabdalla added enhancement New feature or request [prio] high importance of the issue: high priority labels Jan 20, 2022
@ahmadabdalla ahmadabdalla added this to the v 0.4 milestone Jan 20, 2022
@ahmadabdalla ahmadabdalla self-assigned this Jan 20, 2022
@ahmadabdalla
Copy link
Contributor Author

ahmadabdalla commented Jan 25, 2022

Updated all parameter files for the dependency pipeline to use the new namePrefix token for sxx. Encountered two issues within the dependency pipeline yaml: .Platform: Dependencies

Storage Account Upload

# Get storage account name
$parameterFilePath = Join-Path '$(Build.SourcesDirectory)' '$(dependencyPath)' '$(resourceType)' 'parameters' 'parameters.json'
$storageAccountParameters = (ConvertFrom-Json (Get-Content -path $parameterFilePath -Raw)).parameters

Key Vault Secrets

# Get key vault name
$parameterFilePath = Join-Path '$(Build.SourcesDirectory)' '$(dependencyPath)' '$(resourceType)' 'parameters' 'parameters.json'
$keyVaultParameters = (ConvertFrom-Json (Get-Content -Path $parameterFilePath -Raw)).parameters
$keyVaultName = $keyVaultParameters.name.value

They're using the parameter files for the source of the name. Which can't be used before tokens are replaced.

@eriqua .. lets sync up to discuss this soon.

@eriqua
Copy link
Contributor

eriqua commented Jan 26, 2022

Hey @ahmadabdalla, makes sense. I'm using the same method for retrieving the image template name in a new addition to the dependency pipeline. That would fail too.
The first thing I could think of is to leverage the token replacement in the failing PS job, right before retrieving the parameter values. Probably adding a PS task with something like this snippet would be enough

        # Load used functions
        . (Join-Path $env:GITHUB_WORKSPACE 'utilities' 'pipelines' 'tokensReplacement' 'Convert-TokensInParameterFile.ps1')

        # Load Settings File
        $Settings = Get-Content -Path "settings.json" | ConvertFrom-Json
        $ConvertTokensInputs = @{
            ParameterFilePath                 = '${{ inputs.parameterFilePath }}'
            LocalCustomParameterFileTokens    = $Settings.parameterFileTokens.localTokens.tokens
            TokenPrefix                       = $Settings.parameterFileTokens.tokenPrefix
            TokenSuffix                       = $Settings.parameterFileTokens.tokenSuffix
        }
        # Invoke Token Replacement Functionality
        $null = Convert-TokensInParameterFile @ConvertTokensInputs -Verbose

What do you think?

@ahmadabdalla
Copy link
Contributor Author

@eriqua .. yep that's what I was going to with. Thanks for the image note as well :) There's also a static guid that I saw, here. Is that a generic GUID? Or specific? If specific maybe we can follow the same approach to msiPrincipalId?

@eriqua
Copy link
Contributor

eriqua commented Jan 26, 2022

@ahmadabdalla that's the object Id for the tenant Backup Management Service. It's tenant specific unfortunately so we cannot retrieve it automatically unless we add AAD rights to the SPN I guess. It's not generated by a module so we cannot leverage the output as done with the msiPrincipalId approach.
We could investigate on the alternative to use RBAC on the KV instead of Access policies though..

@ahmadabdalla
Copy link
Contributor Author

@eriqua , we'll think through how to resolve for the backup management service. In the meantime, this issue will have a dependency on #929

@eriqua
Copy link
Contributor

eriqua commented Jan 27, 2022

Yes @ahmadabdalla, for the topic at hand we can apply the same solution to the updated dependency pipeline as well. So if it works already for storage and keyvault it should be a quick addition

@ahmadabdalla
Copy link
Contributor Author

New Issue created for Backup Identity - Object ID #937 so that we don't solve for different problems in one issue. Thanks @eriqua

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request [prio] high importance of the issue: high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants