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

Update docs on Azure identity #1167

Merged
merged 1 commit into from
Aug 15, 2023
Merged

Conversation

somtochiama
Copy link
Member

No description provided.

@somtochiama somtochiama requested a review from hiddeco July 14, 2023 01:08
@@ -233,6 +233,12 @@ by extension gain access to ACR.
When the kubelet managed identity has access to ACR, source-controller running on
it will also have access to ACR.

*Note*: If you have more identity configured on the cluster, you have to specify which one to use
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*Note*: If you have more identity configured on the cluster, you have to specify which one to use
*Note*: If you have more than one identity configured on the cluster, you have to specify which one to use

To use Workload Identity, you have to install the Workload Identity
mutating webhook and create an identity that has access to ACR. Next, establish
To use Workload Identity, the Workload Identity mutating webhook has to be installed on your cluster and
you have tocreate an identity that has access to ACR. Next, establish
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
you have tocreate an identity that has access to ACR. Next, establish
you have to create an identity that has access to ACR. Next, establish

@@ -224,7 +224,7 @@ to the IAM role when using IRSA.

#### Azure

The `azure` provider can be used to authenticate automatically using kubelet managed
The `azure` provider can be used to authenticate automatically using workload identity, kubelet managed
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `azure` provider can be used to authenticate automatically using workload identity, kubelet managed
The `azure` provider can be used to authenticate automatically using Workload Identity, kubelet managed

a federated identity between the source-controller ServiceAccount and the
identity. Patch the source-controller Pod and ServiceAccount as shown in the patch
above. Please take a look at this [guide](https://azure.github.io/azure-workload-identity/docs/quick-start.html#6-establish-federated-identity-credential-between-the-identity-and-the-service-account-issuer--subject).

##### AAD Pod Identity
##### AAD Pod Identity - Deprecated!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
##### AAD Pod Identity - Deprecated!
##### Deprecated: AAD Pod Identity

docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
@darkowlzz
Copy link
Contributor

Also, these changes would apply to OCIRepository and Bucket spec docs too.

docs/spec/v1beta2/buckets.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/ocirepositories.md Outdated Show resolved Hide resolved
docs/spec/v1beta2/helmrepositories.md Show resolved Hide resolved
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!

Maybe squash before merge as there are a few minor mixed commits.

##### AAD Pod Identity
##### Deprecated: AAD Pod Identity

**Note:** The AAD Pod Identity project will be archived in [September 2023](https://github.com/Azure/aad-pod-identity#-announcement),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have warnings instead of notes for deprecation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Since these docs are used in the docs site, it may be better to use all the shortcodes. But then that'll not be a standard markdown document and I think at present, the spec docs are just standard markdown documents.
We may have to collectively decide what to do about it and update all the docs to use the shortcodes if we go with the docs style guide.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have changed it to use markdown with Warning instead

@stefanprodan stefanprodan added the area/docs Documentation related issues and pull requests label Aug 1, 2023
Signed-off-by: Somtochi Onyekwere <[email protected]>
@stefanprodan stefanprodan merged commit aa370f2 into fluxcd:main Aug 15, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Documentation related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants