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

Rename auto-tls to external-domain-tls #14482

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

ReToCode
Copy link
Member

@ReToCode ReToCode commented Oct 6, 2023

Fixes #14369

Proposed Changes

  • Renames all occurrences of auto-tls to external-domain-tls
  • Updates the README to run external-domain-tls tests locally

Release Note
Is covered in #14472

Followup work

  • Change autotls targets (params) in knative/infra
  • Revert the reverse compatibility after changes in knative/infra

@knative-prow knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. area/API API objects and controllers area/networking area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Oct 6, 2023
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3eb979a) 86.04% compared to head (12f6748) 86.04%.
Report is 34 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14482   +/-   ##
=======================================
  Coverage   86.04%   86.04%           
=======================================
  Files         196      196           
  Lines       14880    14887    +7     
=======================================
+ Hits        12803    12810    +7     
  Misses       1764     1764           
  Partials      313      313           
Files Coverage Δ
pkg/activator/config/store.go 100.00% <100.00%> (ø)
pkg/apis/serving/v1/route_lifecycle.go 100.00% <ø> (ø)
...kg/apis/serving/v1beta1/domainmapping_lifecycle.go 81.25% <ø> (ø)
pkg/reconciler/domainmapping/reconciler.go 93.68% <100.00%> (ø)
pkg/reconciler/route/route.go 79.70% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ReToCode
Copy link
Member Author

ReToCode commented Oct 6, 2023

/test https

2 similar comments
@ReToCode
Copy link
Member Author

ReToCode commented Oct 9, 2023

/test https

@ReToCode
Copy link
Member Author

ReToCode commented Oct 9, 2023

/test https

@ReToCode
Copy link
Member Author

ReToCode commented Oct 9, 2023

/test https

@knative-prow
Copy link

knative-prow bot commented Oct 9, 2023

@ReToCode: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
https_serving_main 12f6748 link false /test https

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.

@ReToCode
Copy link
Member Author

ReToCode commented Oct 9, 2023

Not sure if the https errors are really related to the changes here. Tests seem not to work at all (https://testgrid.k8s.io/r/knative-own-testgrid/serving#https).

WDYT? @dprotaso @nak3

@nak3
Copy link
Contributor

nak3 commented Oct 11, 2023

You are correct. HTTPS test is not stable now #14435

@nak3
Copy link
Contributor

nak3 commented Oct 11, 2023

/lgtm
/approve

I think I don't have a power to approve though.

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

@dprotaso would you mind taking a look ^^

Copy link
Member

@dprotaso dprotaso left a comment

Choose a reason for hiding this comment

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

Added some minor comments that can be done in a follow up

Copy link
Member

Choose a reason for hiding this comment

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

Can you make a follow up issue so we don't forget to remove this file after the infra changes?

Copy link
Member

Choose a reason for hiding this comment

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

rename filename in follow up

@knative-prow
Copy link

knative-prow bot commented Oct 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, nak3, 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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2023
@dprotaso
Copy link
Member

/test https_serving_main

@knative-prow knative-prow bot merged commit 2c0b8dc into knative:main Oct 20, 2023
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. area/API API objects and controllers area/networking area/test-and-release It flags unit/e2e/conformance/perf test issues for product features lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the aligned flags and certificate secrets in all components
4 participants