Allow disabling AutoTLS with an annotation #9194

arturenault · 2020-08-27T23:48:13Z

/lint

Release Note

Add the ability to disable AutoTLS on a per-route basis using the networking.knative.dev/disableAutoTLS. When set to "true", the annotation will force AutoTLS to be disabled on the route, regardless of cluster configuration.

Pending on knative/networking#124.

knative-prow-robot · 2020-08-27T23:48:19Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

knative-prow-robot

@arturenault: 0 warnings.

In response to this:

/lint

Fixes #8567

Release Note

Add the ability to disable AutoTLS on a per-route basis using the networking.knative.dev/disableAutoTLS. When set to "true", the annotation will force AutoTLS to be disabled on the route, regardless of cluster configuration.

Pending on knative/networking#124.

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/test-infra repository.

pkg/apis/serving/v1alpha1/route_validation.go

pkg/apis/serving/v1alpha1/route_validation_test.go

pkg/reconciler/route/route_test.go

pkg/apis/serving/v1alpha1/route_validation.go

vendor/knative.dev/networking/pkg/apis/networking/register.go

mattmoor

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

mattmoor

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

googlebot · 2020-08-31T22:27:20Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

vagababov · 2020-09-01T15:56:23Z

@googlebot I consent.

Co-authored-by: Victor Agababov <vagababov@gmail.com>

pkg/reconciler/route/route.go

vagababov · 2020-09-02T19:19:08Z

pkg/reconciler/route/route.go

+			networking.DisableAutoTLSAnnotationKey, annotationValue)
+	}
+
+	return config.FromContext(ctx).Network.AutoTLS && !disabledByAnnotation


Should we check this from the beginning? Why do all the parsing etc, if this is already false?

yeah good point. i'll restructure this

Co-authored-by: Victor Agababov <vagababov@gmail.com>

vagababov · 2020-09-02T20:23:55Z

@googlebot I consent.

vagababov · 2020-09-02T20:25:17Z

/lgtm

googlebot · 2020-09-02T20:25:21Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

knative-metrics-robot · 2020-09-02T20:25:46Z

The following is the coverage report on the affected files.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/route/route.go	84.5%	85.2%	0.7

knative-prow-robot · 2020-09-02T20:35:57Z

@arturenault: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-knative-serving-contour-latest	`e8a3923`	link	`/test pull-knative-serving-contour-latest`
pull-knative-serving-istio-stable-mesh	`e8a3923`	link	`/test pull-knative-serving-istio-stable-mesh`

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

ZhiminXiang · 2020-09-02T20:46:42Z

/lgtm
/approve

knative-prow-robot · 2020-09-02T20:47:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arturenault, ZhiminXiang

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:

~~pkg/apis/OWNERS~~ [ZhiminXiang]
~~pkg/reconciler/route/OWNERS~~ [ZhiminXiang]

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 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 27, 2020

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/API API objects and controllers labels Aug 27, 2020

knative-prow-robot reviewed Aug 27, 2020

View reviewed changes

knative-prow-robot added the area/networking label Aug 27, 2020

knative-prow-robot requested review from mdemirhan and ZhiminXiang August 27, 2020 23:48

nak3 assigned ZhiminXiang Aug 27, 2020

vagababov reviewed Aug 28, 2020

View reviewed changes

julz reviewed Aug 28, 2020

View reviewed changes

pkg/apis/serving/v1alpha1/route_validation.go Outdated Show resolved Hide resolved

vendor/knative.dev/networking/pkg/apis/networking/register.go Outdated Show resolved Hide resolved

arturenault changed the title ~~Allow disabling AutoTLS with an annotation~~ [WIP] Allow disabling AutoTLS with an annotation Aug 28, 2020

arturenault mentioned this pull request Aug 28, 2020

Update dependencies #9201

Closed

mattmoor reviewed Aug 31, 2020

View reviewed changes

googlebot added cla: no Indicates the PR's author has not signed the CLA. and removed cla: yes Indicates the PR's author has signed the CLA. labels Aug 31, 2020

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 31, 2020

arturenault force-pushed the disableAutoTLS branch 3 times, most recently from 4dc0198 to a10d868 Compare August 31, 2020 22:53

arturenault marked this pull request as ready for review September 1, 2020 16:21

Artur Renault and others added 11 commits September 2, 2020 19:04

Rename

fe82fb8

parsebool

fe0f1e3

parsebool

ed45886

Fix formatting

16d2e2d

Undo changes to vendor files

7b6d241

Update pkg/apis/serving/v1alpha1/route_validation.go

2b82799

Co-authored-by: Victor Agababov <vagababov@gmail.com>

Chain

4cd012b

Use parsebool

e0d81ad

Add todo

9a79c6c

Add a test

1568bcd

Lowercase

5d4647c

arturenault force-pushed the disableAutoTLS branch from 64b48f1 to 5d4647c Compare September 2, 2020 19:07

vagababov reviewed Sep 2, 2020

View reviewed changes

arturenault and others added 4 commits September 2, 2020 15:07

Update pkg/reconciler/route/route.go

937a4b9

Co-authored-by: Victor Agababov <vagababov@gmail.com>

one missing thing

49ca412

rethink logic to save work

2d60492

Formatting

c69081f

vagababov added cla: yes Indicates the PR's author has signed the CLA. and removed cla: no Indicates the PR's author has not signed the CLA. labels Sep 2, 2020

knative-prow-robot assigned vagababov Sep 2, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2020

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2020

knative-prow-robot merged commit 7bf7064 into knative:master Sep 2, 2020

Allow disabling AutoTLS with an annotation #9194

Allow disabling AutoTLS with an annotation #9194

Uh oh!

Conversation

arturenault commented Aug 27, 2020

Uh oh!

knative-prow-robot commented Aug 27, 2020

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

googlebot commented Aug 31, 2020

Uh oh!

vagababov commented Sep 1, 2020

Uh oh!

Uh oh!

vagababov Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

arturenault Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

vagababov commented Sep 2, 2020

Uh oh!

vagababov commented Sep 2, 2020

Uh oh!

googlebot commented Sep 2, 2020

Uh oh!

knative-metrics-robot commented Sep 2, 2020

Uh oh!

knative-prow-robot commented Sep 2, 2020

Uh oh!

ZhiminXiang commented Sep 2, 2020

Uh oh!

knative-prow-robot commented Sep 2, 2020

Uh oh!

Uh oh!