Add support of specifying service tags for Azure cloud provider #61467
Conversation
k8s-ci-robot
added
release-note
feiskyer
requested review from
brendandburns
and removed request for
jdumars and
andyzhangx
k8s-ci-robot
added
the
approved
|
cc @djsly |
|
/retest |
|
/sig azure |
| // supportedServiceTags holds a list of supported service tags on Azure. | ||
| // Refer https://docs.microsoft.com/en-us/azure/virtual-network/security-overview#service-tags for more information. | ||
| supportedServiceTags = sets.NewString("VirtualNetwork", "VIRTUAL_NETWORK", "AzureLoadBalancer", "AZURE_LOADBALANCER", | ||
| "Internet", "INTERNET", "AzureTrafficManager", "Storage", "Sql") |
There was a problem hiding this comment.
AzureTrafficManager, Storage and Sql are still not available in all azure clouds (e.g Germany). If the user would use one of those 3 serviceTag, where would they be provided the error message ?
There was a problem hiding this comment.
@djsly Augmented rules for NSGs has been GA: https://azure.microsoft.com/en-us/updates/agumented-rules-ga-nsg/
There was a problem hiding this comment.
Thanks, I guess since it is GA it is fine to keep them listed, I was jsut wondering what will happen on Clouds that do not yet support them. (e.g. Germany -- see below)
There was a problem hiding this comment.
Azure ARM call will report errors on such case, and users could find the error message in service events (e.g. by kubectl describe service)
feiskyer
force-pushed
the
azure-service-tags
branch
from
c20dc57
to
b7813b1
Compare
|
Pushed a new commit which allows service tags with region ( e.g. |
|
/test pull-kubernetes-e2e-gce |
|
ping @andyzhangx PTAL |
| var sourceAddressPrefixes []string | ||
| if sourceRanges == nil || serviceapi.IsAllowAll(sourceRanges) { | ||
| if (sourceRanges == nil || serviceapi.IsAllowAll(sourceRanges)) && len(serviceTags) == 0 { |
There was a problem hiding this comment.
is there any possiblity that sourceRanges != nil and len(serviceTags) == 0 ?
There was a problem hiding this comment.
Yep, this is expected to do actions in else block
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, feiskyer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all Tests are more than 96 hours old. Re-running tests. |
|
/retest |
|
Automatic merge from submit-queue (batch tested with PRs 61434, 61501, 59609, 61467, 61531). If you want to cherry-pick this change to another branch, please follow the instructions here. |
What this PR does / why we need it:
This PR adds support of specifying service tags for Azure cloud provider by annotation
service.beta.kubernetes.io/azure-allowed-service-tags.Refer https://docs.microsoft.com/en-us/azure/virtual-network/security-overview#service-tags for more information about this feature.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #57914
Special notes for your reviewer:
Release note: