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

OSSM-8296: Add a mTLS configuration doc #449

Closed
wants to merge 2 commits into from
Closed

Conversation

yxun
Copy link

@yxun yxun commented Oct 24, 2024

What type of PR is this?

  • Enhancement / New Feature
  • Bug Fix
  • Refactor
  • Optimization
  • Test
  • Documentation Update

What this PR does / why we need it:

This PR is part of OSSM-8296 Istio Security User Doc topics. It adds a new common doc file about configuring mTLS and concepts using OpenShift Service Mesh.
It also improves User doc explanations according to the existing OpenShift Service Mesh 2.x Security mTLS topic:
ref: https://docs.openshift.com/container-platform/4.17/service_mesh/v2x/ossm-security.html

@istio-testing
Copy link
Collaborator

Hi @yxun. Thanks for your PR.

I'm waiting for a istio-ecosystem or istio 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.

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.

docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
docs/common/security-mTLS-configuration.md Outdated Show resolved Hide resolved
@fjglira
Copy link
Contributor

fjglira commented Oct 28, 2024

Hey @yxun I added some comments, also I think we should reference this documentation in the main doc to be able to get views there

@fjglira
Copy link
Contributor

fjglira commented Oct 28, 2024

I just saw this PR in midstream repo: openshift-service-mesh#159. Make sense to have the doc here also?

@yxun
Copy link
Author

yxun commented Oct 28, 2024

I just saw this PR in midstream repo: openshift-service-mesh#159. Make sense to have the doc here also?

Hello @fjglira , Which target repo should I create this PR to ? the [openshift-service-mesh:main](https://github.com/openshift-service-mesh/sail-operator/tree/main) or the [istio-ecosystem:main](https://github.com/istio-ecosystem/sail-operator/tree/main)

I am not familiar with the automation between those two. I assume I only need to target one and then some automation will apply the doc change to the other one , right ?
Is this the upstream of the doc change or is this the downstream ?

@fjglira
Copy link
Contributor

fjglira commented Oct 28, 2024

I just saw this PR in midstream repo: openshift-service-mesh#159. Make sense to have the doc here also?

Hello @fjglira , Which target repo should I create this PR to ? the [openshift-service-mesh:main](https://github.com/openshift-service-mesh/sail-operator/tree/main) or the [istio-ecosystem:main](https://github.com/istio-ecosystem/sail-operator/tree/main)

I am not familiar with the automation between those two. I assume I only need to target one and then some automation will apply the doc change to the other one , right ? Is this the upstream of the doc change or is this the downstream ?

Hey, we have a sync job that takes all the changes from the istio-ecosystem org to the openshift-service-mesh.. So, if this documentation is useful for the community (I think it is), it is ok only to create the PR here because it will also be merged in the midstream repo. We will need to mention to our docs folks the changes needed to take this docs to the openshift docs later (For example: replace kubectl for oc)

Signed-off-by: Yuanlin Xu <[email protected]>
Copy link
Contributor

@fjglira fjglira left a comment

Choose a reason for hiding this comment

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

LGTM, only one small change. Also I think you should reference this document in the main document, take this example from gateway

mode: STRICT
EOF
```
a. Replace <namespace> with the namespace where the service is located.
Copy link
Contributor

Choose a reason for hiding this comment

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

Only small change here: you can remove the two time that is mentioned this and replace it at the end with: Note: replace <namespace> with the name of the namespace where the service is located

@fjglira
Copy link
Contributor

fjglira commented Oct 29, 2024

/ok-to-test

@dgn
Copy link
Collaborator

dgn commented Oct 29, 2024

Hey @yxun, as this isn't really related to sail-operator, maybe this should go into the openshift-service-mesh org instead?

@yxun
Copy link
Author

yxun commented Oct 30, 2024

Hey @yxun, as this isn't really related to sail-operator, maybe this should go into the openshift-service-mesh org instead?

right, it's more about istio configuration not about operator configuration. i will close this PR and open an OSM org repo PR today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants