Creating VM Modifies Existing Storage Account?

ghost commented 3 years ago

Version report

Jenkins and plugins versions report:

Jenkins: 2.249.1
OS: Linux - 4.15.0-1111-azure
---
azure-credentials:4.0.2
bitbucket:1.1.25
plugin-util-api:1.2.5
matrix-auth:2.6.3
ivy:2.1
matrix-project:1.17
github-api:1.116
git-parameter:0.9.13
lockable-resources:2.8
gradle:1.36
ws-cleanup:0.38
token-macro:2.12
workflow-cps:2.83
mercurial:2.10
pipeline-model-definition:1.7.2
github-branch-source:2.9.0
pipeline-milestone-step:1.3.1
docker-commons:1.17
pipeline-stage-step:2.5
pipeline-model-api:1.7.2
extensible-choice-parameter:1.7.0
maven-plugin:3.7
jquery-detached:1.2.1
jdk-tool:1.4
jquery3-api:3.5.1-1
mailer:1.32.1
ssh-credentials:1.18.1
git-client:3.4.2
pipeline-stage-tags-metadata:1.7.2
jquery:1.12.4-1
trilead-api:1.0.10
cloud-stats:0.25
pipeline-build-step:2.13
structs:1.20
config-file-provider:3.6.3
antisamy-markup-formatter:2.1
github:1.31.0
conditional-buildstep:1.3.6
durable-task:1.35
handy-uri-templates-2-api:2.1.8-1.0
credentials:2.3.13
command-launcher:1.4
plain-credentials:1.7
workflow-support:3.5
timestamper:1.11.5
okhttp-api:3.14.9
pipeline-graph-analysis:1.10
throttle-concurrents:2.0.3
apache-httpcomponents-client-4-api:4.5.10-2.0
subversion:2.13.1
credentials-binding:1.23
git-server:1.9
echarts-api:4.8.0-2
handlebars:1.1.1
copyartifact:1.45.1
pipeline-rest-api:2.15
workflow-multibranch:2.22
workflow-durable-task-step:2.36
run-condition:1.3
workflow-step-api:2.22
htmlpublisher:1.23
cloudbees-bitbucket-branch-source:2.9.4
docker-plugin:1.2.0
gitlab-plugin:1.5.13
authentication-tokens:1.4
workflow-api:2.40
momentjs:1.1.1
azure-commons:1.0.4
branch-api:2.6.0
pipeline-stage-view:2.15
workflow-scm-step:2.11
ace-editor:1.1
javadoc:1.6
script-security:1.74
pam-auth:1.6
jsch:0.1.55.2
workflow-job:2.40
cloudbees-folder:6.14
parameterized-trigger:2.38
workflow-cps-global-lib:2.17
bouncycastle-api:2.18
scm-api:2.6.3
pipeline-input-step:2.12
email-ext:2.76
ant:1.11
docker-workflow:1.25
junit:1.34
artifactory:3.10.4
extended-choice-parameter:0.82
pipeline-github-lib:1.0
mapdb-api:1.0.9.0
jackson2-api:2.11.2
ldap:1.26
snakeyaml-api:1.27.0
ssh-slaves:1.31.2
resource-disposer:0.14
workflow-basic-steps:2.21
git:4.4.2
display-url-api:2.3.3
embeddable-build-status:2.0.3
workflow-aggregator:2.6
pipeline-model-extensions:1.7.2
ssh-agent:1.20
docker-java-api:3.1.5.2
azure-vm-agents:1.5.1
Result: [Plugin:azure-credentials, Plugin:bitbucket, Plugin:plugin-util-api, Plugin:matrix-auth, Plugin:ivy, Plugin:matrix-project, Plugin:github-api, Plugin:git-parameter, Plugin:lockable-resources, Plugin:gradle, Plugin:ws-cleanup, Plugin:token-macro, Plugin:workflow-cps, Plugin:mercurial, Plugin:pipeline-model-definition, Plugin:github-branch-source, Plugin:pipeline-milestone-step, Plugin:docker-commons, Plugin:pipeline-stage-step, Plugin:pipeline-model-api, Plugin:extensible-choice-parameter, Plugin:maven-plugin, Plugin:jquery-detached, Plugin:jdk-tool, Plugin:jquery3-api, Plugin:mailer, Plugin:ssh-credentials, Plugin:git-client, Plugin:pipeline-stage-tags-metadata, Plugin:jquery, Plugin:trilead-api, Plugin:cloud-stats, Plugin:pipeline-build-step, Plugin:structs, Plugin:config-file-provider, Plugin:antisamy-markup-formatter, Plugin:github, Plugin:conditional-buildstep, Plugin:durable-task, Plugin:handy-uri-templates-2-api, Plugin:credentials, Plugin:command-launcher, Plugin:plain-credentials, Plugin:workflow-support, Plugin:timestamper, Plugin:okhttp-api, Plugin:pipeline-graph-analysis, Plugin:throttle-concurrents, Plugin:apache-httpcomponents-client-4-api, Plugin:subversion, Plugin:credentials-binding, Plugin:git-server, Plugin:echarts-api, Plugin:handlebars, Plugin:copyartifact, Plugin:pipeline-rest-api, Plugin:workflow-multibranch, Plugin:workflow-durable-task-step, Plugin:run-condition, Plugin:workflow-step-api, Plugin:htmlpublisher, Plugin:cloudbees-bitbucket-branch-source, Plugin:docker-plugin, Plugin:gitlab-plugin, Plugin:authentication-tokens, Plugin:workflow-api, Plugin:momentjs, Plugin:azure-commons, Plugin:branch-api, Plugin:pipeline-stage-view, Plugin:workflow-scm-step, Plugin:ace-editor, Plugin:javadoc, Plugin:script-security, Plugin:pam-auth, Plugin:jsch, Plugin:workflow-job, Plugin:cloudbees-folder, Plugin:parameterized-trigger, Plugin:workflow-cps-global-lib, Plugin:bouncycastle-api, Plugin:scm-api, Plugin:pipeline-input-step, Plugin:email-ext, Plugin:ant, Plugin:docker-workflow, Plugin:junit, Plugin:artifactory, Plugin:extended-choice-parameter, Plugin:pipeline-github-lib, Plugin:mapdb-api, Plugin:jackson2-api, Plugin:ldap, Plugin:snakeyaml-api, Plugin:ssh-slaves, Plugin:resource-disposer, Plugin:workflow-basic-steps, Plugin:git, Plugin:display-url-api, Plugin:embeddable-build-status, Plugin:workflow-aggregator, Plugin:pipeline-model-extensions, Plugin:ssh-agent, Plugin:docker-java-api, Plugin:azure-vm-agents]

What Operating System are you using (both controller, and any agents involved in the problem)?

Ubuntu 16.04

Reproduction steps

Step 1: Configure the plugin to use existing resource group and storage account
Step 2: Configure storage account to disable public blob access
Step 3: Have an Azure policy that forbids public blob access
Step 4: Run a job, triggering Azure Agents VM plugin to create a VM

Results

Expected result: a VM is created and the build is run

Actual result: a VM is not created. Azure blocks an attempt to set the storage account to enable public blob access (log attached) AzureError.txt

timja commented 3 years ago

How are you doing private access service endpoint or private endpoint?

timja commented 3 years ago

I’ll try take a look at this tomorrow, btw your policy reason is misspelling anonymous:

anynamous

ghost commented 3 years ago

Service endpoint - Microsoft.Storage

timja commented 3 years ago

So two options that I can see:

We add a checkbox that controls the allowBlobPublicAccess (which we aren't setting but it's using the default).
Evaluate whether we need the storage account still, I think a number of options don't actually need it, only things like unmanaged disks

two would be better I think

ghost commented 3 years ago

Yeah for what it's worth, I have configured it to use managed disks though I confess I don't quite understand the distinction and just went with the recommended setting.

If requiring a storage account, I'd love to see a checkbox that directly controls that Azure setting. I still don't understand why it's attempting to do anything to an existing storage account. I did have a hunch that 'Enabled' was the default in Azure because I couldn't make sense of what was happening otherwise.

If managed disks aren't supposed to be 'using' the storage account anyway, why does it seem to be trying to set that field on my storage account? Just curious - I was digging through the plugin code to try and figure out what was going on and didn't make much headway.

In the meantime, I'd love to simply patch the plugin myself but I haven't been able to figure out where to shove 'storage account allow blob public access = false'

timja commented 3 years ago

Yeah it’s not the simplest patch

it’s just running an ARM template which is unfortunately not leaving unspecified options alone, it’s questionable whether the ‘existing’ storage account should be including the storage account resource, the only reason I can see is maybe it gets an output from it but I doubt it

so that’s another option is refactor it to not touch the storage account if it’s existing

the problem with this change is there’s lots of duplicated arm templates doing slightly different things.

I would rather not add a flag for this as it’s just another setting for users

ideally we

don’t have a storage account if not needed
Don’t touch existing accounts

timja commented 3 years ago

Ok I think we need the storage account for uploading the custom script.

So the fix should be if storage account is set to existing then we don't touch it

dojones1 commented 3 years ago

Have just fallen over this due to an organisationally imposed policy change last week. Had to drop back to a static SSH agent in the meantime.

The storage account == existing -> do nothing would work for us, However there is still an issue with create new storage account violating the policy, and hence now needing to specify the allowBlobPublicAccess == false (or at least configurable)

timja commented 3 years ago

PR welcome if someone tests it and it works for them...

Ideally it defaults to off.

I'm not 100% sure if we rely on the public access for any functionality...

I'm currently busy elsewhere upgrading all the SDKs so won't have time to get to this for a bit.

mattigot commented 3 years ago

Hey @timja

You closed my tickit #280 I see this is the same issue

but i dont understand the solution

we are using an existing storage. So you wrote: “ So the fix should be if storage account is set to existing then we don't touch it”

but the plugin does try to touch it

please explain the solution :)

thanks

timja commented 3 years ago

umm, it's still what I said before?

This issue is open, which means not fixed...

mattigot commented 3 years ago

Ok. is there an estimation for when it will be fixed? (Even a W/A)

timja commented 3 years ago

SDK upgrades have finished now, so probably in the next 2 weeks or so.

May be sooner

timja commented 3 years ago

Please try https://github.com/jenkinsci/azure-vm-agents-plugin/releases/tag/780.v50d067d02f76

cobblermck commented 3 years ago

That has solved my storage account issue but now I have a public IP address issue :) Same policy violation but different resource. Bummer.

timja commented 3 years ago

you can use private networking under advanced settings

cobblermck commented 3 years ago

I completely forgot about that. Been spending too much time in Docker Cloud lately. Working great here. Appreciate it!

mattigot commented 3 years ago

Please try https://github.com/jenkinsci/azure-vm-agents-plugin/releases/tag/780.v50d067d02f76

Thanks! works!

jenkinsci / azure-vm-agents-plugin