Skip to content

chore: remove service.beta.kubernetes.io/azure-shared-securityrule#10168

Open
YurDuiachenko wants to merge 1 commit intokubernetes-sigs:masterfrom
YurDuiachenko:cleanup/remove-deprecated-annotation
Open

chore: remove service.beta.kubernetes.io/azure-shared-securityrule#10168
YurDuiachenko wants to merge 1 commit intokubernetes-sigs:masterfrom
YurDuiachenko:cleanup/remove-deprecated-annotation

Conversation

@YurDuiachenko
Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Removes the deprecated annotation service.beta.kubernetes.io/azure-shared-securityrule

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

NONE

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 9, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @YurDuiachenko. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 9, 2026
@github-actions github-actions Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Apr 9, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: YurDuiachenko
Once this PR has been reviewed and has the lgtm label, please assign bridgetkromhout for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 9, 2026
@YurDuiachenko
Copy link
Copy Markdown
Contributor Author

closes #9905

@YurDuiachenko
Copy link
Copy Markdown
Contributor Author

/assign nilo19

@YurDuiachenko
Copy link
Copy Markdown
Contributor Author

/assign JoelSpeed

@YurDuiachenko
Copy link
Copy Markdown
Contributor Author

YurDuiachenko commented Apr 9, 2026

also removed from docs MicrosoftDocs/azure-aks-docs#410

@andyzhangx andyzhangx requested a review from Copilot April 22, 2026 08:36
Copy link
Copy Markdown
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the deprecated service.beta.kubernetes.io/azure-shared-securityrule Service annotation and all in-repo references to it, including related helper logic and test coverage.

Changes:

  • Deleted the ServiceAnnotationSharedSecurityRule constant and its documentation.
  • Removed useSharedSecurityRule(...) and simplified getSecurityRuleName(...) to always use the per-service rule prefix.
  • Updated unit/e2e tests to stop setting or validating the deprecated annotation behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/e2e/network/ensureloadbalancer.go Stops setting the deprecated shared-security-rule annotation in the e2e scenario.
pkg/provider/azure_standard_test.go Removes the unit test case that depended on the shared-rule naming behavior.
pkg/provider/azure_standard.go Simplifies security rule naming by removing the shared-rule branch.
pkg/provider/azure_loadbalancer.go Deletes the now-unused useSharedSecurityRule helper.
pkg/consts/consts.go Removes the deprecated annotation constant and associated comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andyzhangx
Copy link
Copy Markdown
Member

@nilo19 can you take a look?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/consts/consts.go
Comment on lines 267 to 273
// ServiceAnnotationDNSLabelName is the annotation used on the service
// to specify the DNS label name for the service.
ServiceAnnotationDNSLabelName = "service.beta.kubernetes.io/azure-dns-label-name"

// ServiceAnnotationSharedSecurityRule is the annotation used on the service
// to specify that the service should be exposed using an Azure security rule
// that may be shared with other service, trading specificity of rules for an
// increase in the number of services that can be exposed. This relies on the
// Azure "augmented security rules" feature.
ServiceAnnotationSharedSecurityRule = "service.beta.kubernetes.io/azure-shared-securityrule"

// ServiceAnnotationLoadBalancerResourceGroup is the annotation used on the service
// to specify the resource group of load balancer objects that are not in the same resource group as the cluster.
ServiceAnnotationLoadBalancerResourceGroup = "service.beta.kubernetes.io/azure-load-balancer-resource-group"
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

This change removes support for a previously recognized Service annotation. Even if it was deprecated, this is still a user-facing behavioral change for clusters/services that still set it (it will now be ignored and security rule names will change). Please add an explicit release note and/or upgrade/migration note, or (if desired) retain a compatibility shim that detects the raw annotation key and logs a warning to help users discover the removal.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants