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 documentation about Knative serving encryption #14213

Merged
merged 4 commits into from
Sep 8, 2023

Conversation

ReToCode
Copy link
Member

@ReToCode ReToCode commented Aug 2, 2023

Changes

  • Update the encryption documentation to the latest discussion (the current docs are outdated)
  • Add a diagrams to show the differences between the different parts of Knative Serving encryption

As we had several discussions in the past, this is an try to summarise and visualise how the different Serving encryption workstreams can align. Please note, this ONLY contains the work of the tracking issue #11906 and omits the mTLS stuff (for now). I'll also update the issues we have to hopefully get a more clear roadmap how we can achieve what is described in that document.

PTAL @davidhadas , @KauzClay , @dprotaso , @skonto , @nak3, @rhuss

Copy link
Contributor

@nak3 nak3 left a comment

Choose a reason for hiding this comment

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

LGTM.

I left a few comments. But they are just nit, it is alright without fix them.

docs/encryption/knative-encryption.md Show resolved Hide resolved
docs/encryption/knative-encryption.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.02% 🎉

Comparison is base (e937b62) 86.18% compared to head (049f8c6) 86.21%.
Report is 104 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14213      +/-   ##
==========================================
+ Coverage   86.18%   86.21%   +0.02%     
==========================================
  Files         199      195       -4     
  Lines       14811    14704     -107     
==========================================
- Hits        12765    12677      -88     
+ Misses       1741     1726      -15     
+ Partials      305      301       -4     

see 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.

@krsna-m
Copy link
Contributor

krsna-m commented Aug 14, 2023

/approve
@KauzClay Could you review and give +1 if it looks good to you?

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2023
@krsna-m
Copy link
Contributor

krsna-m commented Aug 14, 2023

/hold
/unapprove

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2023
@krsna-m krsna-m removed approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 14, 2023
@krsna-m
Copy link
Contributor

krsna-m commented Aug 14, 2023

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2023
@ReToCode
Copy link
Member Author

@dprotaso can this be merged? Some Approver magic is needed.

@rhuss
Copy link
Contributor

rhuss commented Aug 22, 2023

Thanks @ReToCode , looks good to me. I think it reflects well the current status and also give an outlook to the parts that are still missing.

@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 29, 2023
@ReToCode
Copy link
Member Author

/assign @dprotaso

@dprotaso
Copy link
Member

Looks good - though I notice the diagrams don't display text well in dark mode. (it's black on black)

@ReToCode
Copy link
Member Author

Looks good - though I notice the diagrams don't display text well in dark mode. (it's black on black)

Good point, backgrounds were transparent, I changed it to white.

@ReToCode
Copy link
Member Author

ReToCode commented Sep 7, 2023

@dprotaso gentle ping

@dprotaso
Copy link
Member

dprotaso commented Sep 8, 2023

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2023
@knative-prow
Copy link

knative-prow bot commented Sep 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, kvmware, ReToCode

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants