Skip to content

Add activator-ca and activator-name keys in config-network#608

Merged
knative-prow-robot merged 2 commits into
knative:mainfrom
nak3:add-ca-config
Feb 8, 2022
Merged

Add activator-ca and activator-name keys in config-network#608
knative-prow-robot merged 2 commits into
knative:mainfrom
nak3:add-ca-config

Conversation

@nak3
Copy link
Copy Markdown
Contributor

@nak3 nak3 commented Jan 26, 2022

This pach adds activator-ca and activator-name keys in config-network.

Part of knative-extensions/net-kourier#750
knative-extensions/net-kourier#761 demonstrates how it works.

/cc @evankanderson @julz @carlisia

This pach adds `activator-ca` and `activator-name` keys in `config-network`.

Part of knative-extensions/net-kourier#750
knative-extensions/net-kourier#761 demonstrates how it works.
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 26, 2022
@knative-prow-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3

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

The pull request process is described here

Details 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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 26, 2022

Codecov Report

Merging #608 (af5ee0c) into main (8acb8e7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #608      +/-   ##
==========================================
+ Coverage   94.76%   94.78%   +0.02%     
==========================================
  Files          38       38              
  Lines         802      806       +4     
==========================================
+ Hits          760      764       +4     
  Misses         30       30              
  Partials       12       12              
Impacted Files Coverage Δ
pkg/network.go 86.95% <100.00%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8acb8e7...af5ee0c. Read the comment docs.

@nak3
Copy link
Copy Markdown
Contributor Author

nak3 commented Jan 26, 2022

Although I added the name with activator-* based on knative-extensions/net-kourier#750 (comment), I wonder if we should use upstream-ca and upstream-name 🤔 We will support the encrypted route between ingress and queue-proxy in the future (I know alpha stage does not support it, though.)

@nak3
Copy link
Copy Markdown
Contributor Author

nak3 commented Feb 7, 2022

/cc @carlisia @ZhiminXiang

Could you please take a look?

Comment thread config/config-network.yaml Outdated
#
# NOTE: This flag is in an alpha state and is mostly here to enable internal testing
# for now. Use with caution.
activator-name: ""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: I slightly prefer activator-san to activator-name as activator-name seems too generic. But if you folks have already decided to use activator-name, it is OK to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, users may misunderstand as activator-name means activator's deployment name or something.
Updated the configuration as activator-san. Thank you.

Copy link
Copy Markdown

@ZhiminXiang ZhiminXiang left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2022
@knative-prow-robot knative-prow-robot merged commit c9e7082 into knative:main Feb 8, 2022
@nak3 nak3 added this to the v1.3.0 milestone Feb 14, 2022
knative-prow Bot pushed a commit that referenced this pull request Apr 12, 2022
* Add certificates keys in config-network

This patch adds the following certificate variables:

- `activator-server-certs`
- `queue-proxy-ca`
- `queue-proxy-san`
- `queue-proxy-server-certs`

It is similar to #608.

knative/serving#12815 and knative/serving#12770
verifeid the change.

* Rename -server-certs with -cert-secret

* Bump checksum

* Fix lint
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.

3 participants