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

Upstream update to v1.3.2 #76

Merged
merged 2 commits into from
Dec 19, 2022
Merged

Upstream update to v1.3.2 #76

merged 2 commits into from
Dec 19, 2022

Conversation

nprokopic
Copy link
Contributor

@nprokopic nprokopic commented Dec 16, 2022

Towards https://github.com/giantswarm/giantswarm/issues/24551

Changes

  • Update upstream cluster-api-provider-azure version from v1.2.1 to v1.3.2 (see highlighted changes below)

Highlighted upstream changes that can be relevant for vintage workload clusters

(with specified upstream cluster-api-provider-azure versions)

  • v1.3.0 Add support for Service Principal with Certificate auth using AAD pod identity. This looks like a breaking change in theory, since AzureClusterIdentity UserAssignedMSI type is removed, but in practice it is not, because UserAssignedMSI never worked, see this comment for more details. In any case Giant Swarm workload clusters are not be affected, because all of them are using ServicePrincipal type (I checked all workload clusters that are deployed at the time of writing on 2022-12-17) and this breaking change is reverted in the next minor release.

Upstream cluster-api-provider-azure release notes

enum:
- ServicePrincipal
- ManualServicePrincipal
- UserAssignedMSI
- ServicePrincipalCertificate
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like a breaking change in theory, but in practice it is not, as UserAssignedMSI never worked, see this comment for more details.

In any case we should not be affected, because all vintage workload clusters are using ServicePrincipal type.

Copy link
Contributor

Choose a reason for hiding this comment

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

just for reference i did test it in capz 1.6.x and it worked for me using the UA assigned to the MC node

apparently the actual issue with UA Identity was fixed in CAPZ 1.4 kubernetes-sigs/cluster-api-provider-azure#1104

👍

@@ -35,6 +35,26 @@
type: string
description: AdditionalTags is an optional set of tags to add to Azure resources managed by the Azure provider, in addition to the ones added by default.
type: object
addonProfiles:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are still not using CAPZ managed clusters, so not affected by this.

@nprokopic nprokopic marked this pull request as ready for review December 17, 2022 13:10
@nprokopic nprokopic requested review from a team December 17, 2022 13:10
@nprokopic nprokopic mentioned this pull request Dec 18, 2022
2 tasks
Copy link

@bavarianbidi bavarianbidi left a comment

Choose a reason for hiding this comment

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

lgtm for CAPI (Clippy)

Copy link
Contributor

@primeroz primeroz left a comment

Choose a reason for hiding this comment

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

LGTM

@nprokopic nprokopic merged commit ae0decb into main Dec 19, 2022
@nprokopic nprokopic deleted the upstream-update-v1.3.2 branch December 19, 2022 10:21
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.

3 participants