Skip to content
This repository was archived by the owner on Oct 24, 2023. It is now read-only.

feat: Consume signed powershell scripts#3441

Merged
jsturtevant merged 14 commits intoAzure:masterfrom
marosset:consumed-signed-powershell
Jul 2, 2020
Merged

feat: Consume signed powershell scripts#3441
jsturtevant merged 14 commits intoAzure:masterfrom
marosset:consumed-signed-powershell

Conversation

@marosset
Copy link
Contributor

@marosset marosset commented Jun 10, 2020

Reason for Change:

Issue Fixed:

Requirements:

Notes:

@marosset marosset changed the title Consumed signed powershell Consume signed powershell scripts Jun 10, 2020
@marosset marosset marked this pull request as draft June 10, 2020 18:19
@marosset
Copy link
Contributor Author

/azp run pr-e2e

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@marosset marosset force-pushed the consumed-signed-powershell branch from a96c825 to cc4828c Compare June 12, 2020 21:25
@marosset marosset marked this pull request as ready for review June 12, 2020 21:25
@marosset marosset changed the title Consume signed powershell scripts feat: Consume signed powershell scripts Jun 15, 2020
pkg/api/const.go Outdated
// WindowsProfile defaults
// TODO: Move other values defined in WindowsProfiles (like DefaultWindowsSSHEnabled) here.
const (
DefaultWindowsProvisioningScriptsPackageURL = "https://kubernetesartifacts.azureedge.net/aks-engine/windows/provisioning/signedscripts-v0.0.1.zip"
Copy link
Member

Choose a reason for hiding this comment

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

Should we add this to DefaultKubernetesSpecConfig in azenvtypes.go?
We need to assure that the new package is ready for AzureChinaCloud. AzureChinaCloud should use https://mirror.azk8s.cn/aks-engine.
I think that we should have a rule to manage so many folders for different packages so we do not need to reconfigure the proxy for AzureChinaCloud.
FYI @andyzhangx

Copy link
Contributor

Choose a reason for hiding this comment

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

@jchauncey would you also sync this https://kubernetesartifacts.azureedge.net/aks-engine to https://kubernetesartifacts.core.chinacloudapi.cn/aks-engine, every bits in https://kubernetesartifacts.azureedge.net should sync to https://kubernetesartifacts.core.chinacloudapi.cn otherwise it may break sth on Azure China.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbelHu @andyzhangx I see if the Azure DevOps release pipeline that this package was uploaded to https://kubernetesartifacts.blob.core.chinacloudapi.cn/aks-engine/windows/provisioning/signedscripts-v0.0.1.zip.

@AbelHu are you saying that we should add an entry to KubernetesSpecConfig that points to https://kubernetesartifacts.azureedge.net/aks-engine by default?
I'm not familiar with how other URLs for AzureChinaCloud are managed by aks-engine.
If you have an example to follow I would appreciate it.

Also @jackfrancis as FYI here

Copy link
Member

Choose a reason for hiding this comment

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

I think that we need to add a new property WindowsProvisioningScriptsPackageURL in KubernetesSpecConfig, set DefaultKubernetesSpecConfig.WindowsProvisioningScriptsPackageURL to "https://kubernetesartifacts.azureedge.net/aks-engine/windows/provisioning/signedscripts-" + SignedWindowsScriptsVer + ".zip" for global Azure and expect to set AzureChinaCloudSpec.KubernetesSpecConfig.WindowsProvisioningScriptsPackageURL to "https://mirror.azk8s.cn/aks-engine/windows/provisioning/signedscripts-" + SignedWindowsScriptsVer + ".zip".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @AbelHu. Let me add that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbelHu @andyzhangx please take a look at the last commit in the PR where I added WindowsProvisioningScriptsPackageURL to kubernetesSpecConfig

@marosset marosset force-pushed the consumed-signed-powershell branch from ea9e31b to bb32b52 Compare June 17, 2020 22:49
@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #3441 into master will increase coverage by 0.01%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3441      +/-   ##
==========================================
+ Coverage   73.17%   73.19%   +0.01%     
==========================================
  Files         147      147              
  Lines       25034    25051      +17     
==========================================
+ Hits        18319    18336      +17     
- Misses       5581     5585       +4     
+ Partials     1134     1130       -4     
Impacted Files Coverage Δ
pkg/api/types.go 94.21% <ø> (-0.21%) ⬇️
pkg/api/vlabs/types.go 71.83% <ø> (ø)
pkg/engine/template_generator.go 81.86% <ø> (-0.03%) ⬇️
pkg/engine/templates_generated.go 53.45% <ø> (-0.63%) ⬇️
pkg/api/defaults.go 92.77% <85.71%> (+0.04%) ⬆️
pkg/api/converterfromapi.go 94.03% <100.00%> (+0.02%) ⬆️
pkg/api/convertertoapi.go 93.54% <100.00%> (+0.02%) ⬆️
pkg/api/defaults-custom-cloud-profile.go 85.93% <100.00%> (+0.11%) ⬆️
pkg/engine/armvariables.go 86.22% <100.00%> (+0.08%) ⬆️
cmd/scale.go 11.92% <0.00%> (-0.27%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff0b7b6...410a368. Read the comment docs.

@acs-bot
Copy link

acs-bot commented Jun 30, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AbelHu, marosset

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ksubrmnn ksubrmnn dismissed their stale review July 1, 2020 16:08

Dismissing changes

@jsturtevant jsturtevant merged commit b457aa6 into Azure:master Jul 2, 2020
penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
* moving some windows powershell scripts to a new staging directory
* powershell updates to download package and add files to c:\k
* removing references to provisioning scripts from c:/azuredata/
* adding validate-windows-provisioning-scripts.sh
* Adding build-windows-provisioning-scripts.sh
* Adding windows-provisioning-scripts.md
* Updating references to a package containing signed powershell files
* ensuring windows scripts package works in azure china cloud
@marosset marosset deleted the consumed-signed-powershell branch November 10, 2020 21:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants