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

Tokens Replacement Service [Pilot] #447

Merged
merged 20 commits into from
Nov 15, 2021
Merged

Tokens Replacement Service [Pilot] #447

merged 20 commits into from
Nov 15, 2021

Conversation

ahmadabdalla
Copy link
Contributor

@ahmadabdalla ahmadabdalla commented Nov 9, 2021

Change #295

First release for the Tokens Replacement Functionality

  • Supports 3 Token Types
  1. Default Tokens [Only modifiable via Environment Variables and Actions]: Comes from environment variables. Defined in the Actions
  2. Local Tokens: [Optional, Stored in Source Control]. Stored in the Settings JSON
  3. Remote Tokens: [Optional, Stored in Key Vault]. Only activated when the PLATFORM_KEYVAULT secret is defined in GitHub.
    3a. ParameterFileTokens: Normal key/value pairs.
    3b. SecureParameterFileTokens: Are always set as 'Secure Strings' and only swapped at the last phase. Should be used for Secure String Parameters.

Please read the following documentations


NOTE: This is not a breaking change and is only applied transparently with no impact to existing resources. Only Resources that want to consume the tokens, can now do it by performing two things:

  • Set 2 Environment Variables in the Workflow. DEPLOYMENT_SP_ID and PLATFORM_KEYVAULT
  • Reference Tokens in the Parameter Files as needed (i.e. <<deploymentSpId>>, <<managementGroupId>>, <<platformKeyVault>>.

Testing

Pilot Modules

Network: Applicationsecuritygroups

Network: Networksecuritygroups

Storage: Storage Account

Dependency Pipeline Changes:

  • Added the two new environment variables (Platform Key Vault, and Deployment SP ID)
  • Added Platform Key Vault Task (Conditional to the Env. Variable)
  • Added Store Parameter File Tokens in Key Vault Task (Conditional to the Env. Variable)

There are failures in the current dependency pipeline due to a change in the child resources for event hub. Not related to this change

Other Notes

Global Modules test references test cases for tenant, management group, principal ID checks. Which have been developed but commented out for this release until we enforce the change on all pipelines.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update (Wiki)

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • My corresponding pipelines / checks run clean and green without any errors or warnings
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (readme)
  • I did format my code

@ahmadabdalla ahmadabdalla self-assigned this Nov 9, 2021
@ahmadabdalla ahmadabdalla added this to the v 0.3 milestone Nov 9, 2021
@ahmadabdalla ahmadabdalla added [cat] pipelines category: pipelines [prio] high importance of the issue: high priority documentation Improvements or additions to documentation labels Nov 9, 2021
@github-actions
Copy link

github-actions bot commented Nov 9, 2021

Unit Test Results

  1 files  ±0    1 suites  ±0   19s ⏱️ +4s
31 tests ±0  31 ✔️ ±0  0 💤 ±0  0 ±0 
34 runs  +1  31 ✔️ ±0  3 💤 +1  0 ±0 

Results for commit 5c9e3e3. ± Comparison against base commit 949f42b.

This pull request removes 31 and adds 31 tests. Note that renamed tests count towards both.
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] All apiVersion properties should be set to a static, hard-coded value
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] All parameters in parameters files exist in template file (deploy.json)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] All required parameters in template file (deploy.json) should exist in parameters files
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] All resources that have a Location property should refer to the Location parameter &#x27;parameters(&#x27;Location&#x27;)&#x27;
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] CUA ID deployment should be present in the template
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] If delete lock is implemented, the template should have a lock parameter with the default value of [NotSpecified]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] Output names should be camel-cased (no dashes or underscores and must start with lower-case letter)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] Parameter files should not contain subscriptionId original value and but a token string
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] Parameter names should be camel-cased (no dashes or underscores and must start with lower-case letter)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Insights/metricAlerts] Standard outputs should be provided (e.g. resourceName, resourceId, resouceGroupName)
…
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] All apiVersion properties should be set to a static, hard-coded value
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] All parameters in parameters files exist in template file (deploy.json)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] All required parameters in template file (deploy.json) should exist in parameters files
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] All resources that have a Location property should refer to the Location parameter &#x27;parameters(&#x27;Location&#x27;)&#x27;
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] CUA ID deployment should be present in the template
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] If delete lock is implemented, the template should have a lock parameter with the default value of [NotSpecified]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] Output names should be camel-cased (no dashes or underscores and must start with lower-case letter)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] Parameter files should not contain the subscriptionId guid
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] Parameter names should be camel-cased (no dashes or underscores and must start with lower-case letter)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Network/applicationSecurityGroups] Standard outputs should be provided (e.g. resourceName, resourceId, resouceGroupName)
…

♻️ This comment has been updated with latest results.

Copy link
Contributor

@segraef segraef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, we have to make sure to include the doco into the Wiki for 0.3/0.4 from an overall perspective. Chatted with @ahmadabdalla regarding that as well already.

@MariusStorhaug MariusStorhaug self-requested a review November 10, 2021 08:04
Copy link
Contributor

@MariusStorhaug MariusStorhaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good work. Some minor things here and there.

Copy link
Contributor

@MariusStorhaug MariusStorhaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing.

MariusStorhaug
MariusStorhaug previously approved these changes Nov 15, 2021
Copy link
Contributor

@MariusStorhaug MariusStorhaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All comments i had have been sorted. For me this is good to go. Approved

Copy link
Contributor

@eriqua eriqua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🆗

@ahmadabdalla ahmadabdalla merged commit 7a9d49b into main Nov 15, 2021
@ahmadabdalla ahmadabdalla deleted the users/ahmad/tokensV3 branch November 15, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[cat] pipelines category: pipelines documentation Improvements or additions to documentation [prio] high importance of the issue: high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants