-
Notifications
You must be signed in to change notification settings - Fork 202
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
Add support for KubernetesConfiguration/FluxConfiguration #4275
Conversation
│ ├── AzureBlob: *Object (9 properties) | ||
│ │ ├── AccountKey: *genruntime.SecretReference | ||
│ │ ├── ContainerName: *string | ||
│ │ ├── LocalAuthRef: *string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea what this is, and if it's a secret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the "Name of a local secret on the Kubernetes cluster to use as the authentication secret rather than the managed or user-provided configuration secrets."
│ ├── GitRepository: *Object (8 properties) | ||
│ │ ├── HttpsCACert: *genruntime.SecretReference | ||
│ │ ├── HttpsUser: *string | ||
│ │ ├── LocalAuthRef: *string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question here
│ │ ├── "Bucket" | ||
│ │ └── "GitRepository" | ||
│ ├── Suspend: *bool | ||
│ ├── SystemData: *Object (6 properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that systemdata is expected to be readonly? That's what https://github.com/Azure/azure-rest-api-specs/blob/main/specification/common-types/resource-management/v6/types.json#L31 says at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just prune this from the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
tc := globalTestContext.ForTest(t) | ||
rg := tc.CreateTestResourceGroupAndWait() | ||
|
||
adminUsername := "adminUser" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not the most important topic in this PR, so feel free to ignore it or maybe we should file a separate test improvement issue, but I think you can create AKS clusters without SSH access, and just specify managedIdentity intead.
Feels cleaner for these cases where don't actually care about SSH access?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea, although bit torn on if it's worth spending time to get it replaced with MI and re-record the tests?
Since it's just a CRUD test for a resource and we have random generated passwords safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just file a low-priority issue for it?
Closes #2312
What this PR does / why we need it:
Add new resource Microsoft.KubernetesConfiguration/FluxConfiguration.
If applicable: