Skip to content

Update PeerAuthentication docs #3184

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 2, 2024
Merged

Update PeerAuthentication docs #3184

merged 6 commits into from
Jul 2, 2024

Conversation

craigbox
Copy link
Contributor

@craigbox craigbox commented May 9, 2024 •
edited
Loading

Given we seem to have decided to keep PeerAuthentication around in ambient mode, update the docs to remove the warning, and add the caveat you can't DISABLE mTLS any more.

(This may or may not be the project's decision.)

@craigbox craigbox requested a review from a team as a code owner May 9, 2024 03:48
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 9, 2024
@craigbox craigbox added the release-notes-none Indicates a PR that does not require release notes. label May 9, 2024
@craigbox craigbox mentioned this pull request May 9, 2024
Closed
howardjohn
howardjohn reviewed May 9, 2024
View reviewed changes
Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

overall LGTM

@craigbox craigbox added the cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch label May 16, 2024
@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 17, 2024
hzxuzhonghu
hzxuzhonghu reviewed May 17, 2024
View reviewed changes
security/v1/peer_authentication.proto Outdated
// In ambient mode, security is transparently enabled for a pod by the ztunnel node agent.
// (Traffic between proxies uses the HBONE protocol, which includes encryption with mTLS.)
// Because of this, `DISABLE` mode is not supported.
// `STRICT` mode is useful to ensure that connections that bypass the mesh are not possible.
Copy link
Member

Choose a reason for hiding this comment

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

could we say this api doesnot apply to ambient mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does apply to ambient mode though, just not with DISABLE mode, AIUI. @keithmattix

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I missed this. This is correct

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label May 20, 2024
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed needs-rebase Indicates a PR needs to be rebased before being merged size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 21, 2024
@craigbox
Copy link
Contributor Author

rebased and hopefully ready for proper merging

@craigbox
Copy link
Contributor Author

@howardjohn for formal approval

@craigbox
Copy link
Contributor Author

the gencheck test is asking me to patch a file which doesn't exist any more, since #3188 made the files into aliases.

However, make gen is failing too:

craigbox@solo-system-craigbox:~/Documents/Projects/istio-api$ make gen
./clean.sh
./gen.sh
2024/05/31 14:27:15 Generating istio.security.v1beta1.RequestAuthentication
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.WorkloadGroup
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.VirtualService
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.DestinationRule
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.WorkloadEntry
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.Sidecar
2024/05/31 14:27:15 Generating istio.networking.v1beta1.ProxyConfig
2024/05/31 14:27:15 Generating istio.security.v1beta1.AuthorizationPolicy
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.ServiceEntry
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.EnvoyFilter
2024/05/31 14:27:15 Generating istio.security.v1beta1.PeerAuthentication
2024/05/31 14:27:15 Generating istio.extensions.v1alpha1.WasmPlugin
2024/05/31 14:27:15 Generating istio.telemetry.v1alpha1.Telemetry
2024/05/31 14:27:15 Generating istio.networking.v1alpha3.Gateway
mesh/v1alpha1/config.proto:1273:12:Field "59" with name "discovery_selectors" on message "MeshConfig" changed type from "k8s.io.apimachinery.pkg.apis.meta.v1.LabelSelector" to "istio.mesh.v1alpha1.LabelSelector".
make[1]: *** [Makefile.core.mk:53: breaking] Error 100
make: *** [gen] Error 2

@ericvn
Copy link

ericvn commented May 31, 2024

/test gencheck

1 similar comment
@craigbox
Copy link
Contributor Author

craigbox commented Jul 2, 2024

/test gencheck

@istio-testing istio-testing merged commit b04c256 into istio:master Jul 2, 2024
5 checks passed
@craigbox craigbox deleted the peer-authn branch July 2, 2024 00:41
@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #3184 failed to apply on top of branch "release-1.22":

Applying: Update PeerAuthentication docs for mTLS
Applying: update
Applying: update text
Applying: made gen
Applying: make gen
Applying: fix gencheck
Using index info to reconstruct a base tree...
A	security/v1/peer_authentication_alias.gen.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): security/v1/peer_authentication_alias.gen.go deleted in HEAD and modified in fix gencheck. Version fix gencheck of security/v1/peer_authentication_alias.gen.go left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0006 fix gencheck
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #3250

@istio-testing istio-testing mentioned this pull request Jul 2, 2024
Closed
craigbox added a commit to craigbox/istio-api that referenced this pull request Jul 2, 2024
@craigbox
manual cherry-pick of istio#3184
eaf8bda
@craigbox craigbox mentioned this pull request Jul 2, 2024
Merged
istio-testing pushed a commit that referenced this pull request Jul 2, 2024
@craigbox
manual cherry-pick of #3184 (#3252)
f011423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@costinm costinm costinm left review comments

@linsun linsun linsun left review comments

@howardjohn howardjohn howardjohn approved these changes

@keithmattix keithmattix keithmattix approved these changes

@hzxuzhonghu hzxuzhonghu hzxuzhonghu approved these changes

Assignees
No one assigned
Labels
cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch release-notes-none Indicates a PR that does not require release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@craigbox @ericvn @istio-testing @costinm @howardjohn @keithmattix @linsun @hzxuzhonghu