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

Remove support for Internet Service Tag for Data Provider IP addresses #2331

Merged

Conversation

craddm
Copy link
Contributor

@craddm craddm commented Dec 6, 2024

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
  • Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

⤴️ Summary

Removes support for the Internet service tag for data provider's IP address. This is not currently supported properly by Azure. Specifically, an NFS_v3 storage account cannot be set up using the IP with its default action being to ALLOW all traffic.

It seems that this can be set after the initial set-up. The following warning is displayed in the portal:

Screenshot 2024-11-27 at 11 56 03

Network access to the storage account seems to work as expected, despite the warning, but it still appears that setting up an account with this setting is blocked at the API level.

🌂 Related issues

Closes #2300

🔬 Tests

Tested locally. Fresh deployment successful, IP addresses on storage accounts correctly set.

@craddm craddm changed the title [WIPRemove support for Internet Service Tag for Data Provider IP addresses [WIP] Remove support for Internet Service Tag for Data Provider IP addresses Dec 6, 2024
Copy link

github-actions bot commented Dec 6, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/config
  config_sections.py
  data_safe_haven/infrastructure/components/composite
  nfsv3_storage_account.py 54
  data_safe_haven/infrastructure/programs/sre
  data.py 114
Project Total  

This report was generated by python-coverage-comment-action

@craddm craddm changed the title [WIP] Remove support for Internet Service Tag for Data Provider IP addresses Remove support for Internet Service Tag for Data Provider IP addresses Dec 6, 2024
@craddm craddm marked this pull request as ready for review December 6, 2024 11:59
@craddm craddm requested a review from a team as a code owner December 6, 2024 11:59
Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

Does the template text and docs need updating to reflect this?

@craddm
Copy link
Contributor Author

craddm commented Dec 6, 2024

Does the template text and docs need updating to reflect this?

Neither the template nor the docs actually mention this being an option

Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of comments which I don't think should block merging.

Could you say more about the testing you did? I think what we want to ensure is that,

  1. Deployment works
  2. The allowed IP addresses are populated correctly

@JimMadge JimMadge merged commit af7d7e9 into alan-turing-institute:develop Dec 9, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data provider address "Internet" does not work
2 participants