search

alan-turing-institute / data-safe-haven

https://data-safe-haven.readthedocs.io
BSD 3-Clause "New" or "Revised" License
61 stars 15 forks source link

Changing admin ip address in config does not update network access to storage accounts #2212

Closed craddm closed 4 days ago

craddm commented 1 month ago

:white_check_mark: Checklist

:computer: System information

:package: Packages

List of packages ```none Paste list of packages here ```

:no_entry_sign: Describe the problem

Changing the admin ip address in the configuration of an SRE that has already been deployed does not allow a new admin to modify infrastructure

dsh sre teardown x or dsh sre deploy x will fail because the admin cannot access files on storage accounts, so pulumi cannot refresh its state before continuing.

The logs show multiple errors like the below

~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_ubuntu_pro.yaml refreshing (0s)                          
 ~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_ubuntu_pro.yaml refreshing (0s) error:           
retrieving blob properties "tasks\\ubuntu_pro.yaml" (container "desiredstate" / account "shgresreimsdesiredstate6"): blobs.Client#GetProperties:       
Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF           
 ~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_ubuntu_pro.yaml **refreshing failed** error:           
retrieving blob properties "tasks\\ubuntu_pro.yaml" (container "desiredstate" / account "shgresreimsdesiredstate6"): blobs.Client#GetProperties:       
Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF           
 ~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_user_config.yaml refreshing (0s)                         
 ~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_user_config.yaml refreshing (0s) error:          
retrieving blob properties "tasks\\user_config.yaml" (container "desiredstate" / account "shgresreimsdesiredstate6"): blobs.Client#GetProperties:      
Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF           
 ~  azure-native:storage:Blob sre_desired_state_blob_desired_state_blob_user_config.yaml **refreshing failed** error:          
retrieving blob properties "tasks\\user_config.yaml" (container "desiredstate" / account "shgresreimsdesiredstate6"): blobs.Client#GetProperties:      
Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF

:steam_locomotive: Workarounds or solutions

The workaround is to manually add the admin IP addresses to all storage accounts through the portal

mattwestby commented 1 month ago

I've not experienced this, i updated the ip addresses though in the SRE config file and then reupload the config file and when you do a redeploy it corrects it in the firewall ist on the storage account.

jemrobinson commented 1 month ago

@mattwestby : Did you do a redeploy from an IP address that was already allowed though?

mattwestby commented 1 month ago

Did you do a redeploy from an IP address that was already allowed though?

No, it wasn’t already in the list no.

jemrobinson commented 1 month ago

And was it an IP address belonging to Azure?

mattwestby commented 1 month ago

No was my personal internet public ip.

From: James Robinson @.> Sent: 01 October 2024 15:34 To: alan-turing-institute/data-safe-haven @.> Cc: Matthew Westby (staff) @.>; Mention @.> Subject: Re: [alan-turing-institute/data-safe-haven] Changing admin ip address in config does not update network access to storage accounts (Issue #2212)

And was it an IP address belonging to Azure?

— Reply to this email directly, view it on GitHubhttps://github.com/alan-turing-institute/data-safe-haven/issues/2212#issuecomment-2386148033, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVWWA2IRZJQU3L35POQY5STZZKXFXAVCNFSM6AAAAABPDP2YO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBWGE2DQMBTGM. You are receiving this because you were mentioned.Message ID: @.***>

This message and any attachment are intended solely for the addressee and may contain confidential information. If you have received this message in error, please contact the sender and delete the email and attachment. Any views or opinions expressed by the author of this email do not necessarily reflect the views of the University of Nottingham. Email communications with the University of Nottingham may be monitored where permitted by law.

craddm commented 1 month ago

For me that is exactly what didn't work. Wondering if it's possible that pulumi was picking up things that needed to change after I'd pulled some recent code updates.

mattwestby commented 1 month ago

Wonder whether this is the same thing that is happening with me and the ssl cert creation, pulumi not always picking up changes?

From: Matt Craddock @.> Sent: 01 October 2024 15:37 To: alan-turing-institute/data-safe-haven @.> Cc: Matthew Westby (staff) @.>; Mention @.> Subject: Re: [alan-turing-institute/data-safe-haven] Changing admin ip address in config does not update network access to storage accounts (Issue #2212)

For me that is exactly what didn't work. Wondering if it's possible that pulumi was picking up things that needed to change after I'd pulled some recent code updates.

— Reply to this email directly, view it on GitHubhttps://github.com/alan-turing-institute/data-safe-haven/issues/2212#issuecomment-2386165358, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVWWA2MPSAZKLFFEKKOL62LZZKXRHAVCNFSM6AAAAABPDP2YO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBWGE3DKMZVHA. You are receiving this because you were mentioned.Message ID: @.***>

This message and any attachment are intended solely for the addressee and may contain confidential information. If you have received this message in error, please contact the sender and delete the email and attachment. Any views or opinions expressed by the author of this email do not necessarily reflect the views of the University of Nottingham. Email communications with the University of Nottingham may be monitored where permitted by law.

JimMadge commented 1 month ago

@craddm Can you clarify the steps you took here?

If you are saying you can't refresh from an IP that is not allowed, then I think that is not a bug. If you are saying that it isn't possible to update the allowed IP addresses, then that does sound like a bug.

craddm commented 1 month ago

@JimMadge

1) Deployed SRE from one IP address 2) Tried to teardown SRE from a different IP address, which failed because it wasn't an allowed IP address 3) Added the IP address in the config for the SRE and uploaded the new config 4) Tried to teardown the SRE again, which failed with the errors reported above (which are because the IP address was not allowed on the storage account) 5) Tried to run dsh sre deploy x, in the hope it would update the IP addresses on the storage accounts, which failed with the errors reported above 6) Tore down the SRE from the original IP address

JimMadge commented 1 month ago

I think that is working as expected. The change to the allowed IP address was never applied, just uploaded to the remote configuration.

craddm commented 1 month ago

I think that is working as expected. The change to the allowed IP address was never applied, just uploaded to the remote configuration.

So how should the change to the allowed IP address be applied?

JimMadge commented 1 month ago

dsh config upload ... && dsh sre deploy .... In the same way as all changes.

craddm commented 1 month ago

dsh config upload ... && dsh sre deploy .... In the same way as all changes.

As per step 5, I got the same error from running dsh sre deploy ...

It may be that you have to run this from the original IP address first, which should be documented if so

JimMadge commented 1 month ago

Yes, that is what I mean, that is expected. The infrastructure hasn't been updated to allow access to the storage accounts from the new IP address, we should expect deploy/teardown to fail when Pulumi attempts to modify the blobs/blob containers/etc..

I feel like what you are saying is the same as "I can't access a storage account from an IP address I've blocked", so this all feels quite natural to me.

What do you think should be different?

craddm commented 1 month ago

Yes, that is what I mean, that is expected. The infrastructure hasn't been updated to allow access to the storage accounts from the new IP address, we should expect deploy/teardown to fail when Pulumi attempts to modify the blobs/blob containers/etc..

I feel like what you are saying is the same as "I can't access a storage account from an IP address I've blocked", so this all feels quite natural to me.

What do you think should be different?

You just said that I should have done dsh config upload ... && dsh sre deploy .... to apply the change to the allowed IP addresses. I tried that, and it failed. It never updated the allowed IP addresses. It never got that far, failing because it tried to modify things on the storage accounts before it changed the allowed IP addresses.

If the point is that I should have changed the config and then deployed from the original IP address, then yes, and that should be made clearer in the documentation. A naive user might think "well I changed what IP addresses are allowed so why isn't it working?"

mattwestby commented 1 month ago

when you do the dsh config upload does it show you its removing the "old" ip address and adding the "new" one? Are you making the change in the sre yaml config file first?

craddm commented 1 month ago

when you do the dsh config upload does it show you its removing the "old" ip address and adding the "new" one? Are you making the change in the sre yaml config file first?

I added the new address but didn't remove the old ip address.

mattwestby commented 1 month ago

ah ok in my config file i only had 1 ip address allowed.

JimMadge commented 1 month ago

If the point is that I should have changed the config and then deployed from the original IP address, then yes, and that should be made clearer in the documentation. A naive user might think "well I changed what IP addresses are allowed so why isn't it working?"

OK, I think we are starting to go around in circles. I don't find anything here confusing or surprising,

So I'm not sure what you think should be different (or if you think this is a bug), is it

craddm commented 1 month ago
  • Some documentation (in which case, what does it say?)

"When changing the permitted admin or data owner IP addresses for a deployed SRE in the SRE configuration file, you must run dsh sre deploy ... from the originally permitted IP address after uploading the updated configuration file. You cannot update the allowed IP addresses from the new IP address."

craddm commented 1 month ago

I think my point is that ok, this is intended behaviour, but it isn't completely obvious to anyone who isn't very dsh-brained.

JimMadge commented 1 month ago

@craddm OK, can you open a PR.

We might want to be careful with the wording to balance being helpful with not being misleading. I don't think what you've said above is quite correct.

It might help to add a section (if there isn't one already) explaining the workflow of updating an SRE. Uploading a configuration does not apply any changes itself, only when running deploy.