Skip to content

Adds autest to test error cases loading ssl_multicert.config#8532

Merged
randall merged 1 commit intoapache:masterfrom
randall:autest_cert_loading
Nov 23, 2021
Merged

Adds autest to test error cases loading ssl_multicert.config#8532
randall merged 1 commit intoapache:masterfrom
randall:autest_cert_loading

Conversation

@randall
Copy link
Contributor

@randall randall commented Nov 19, 2021

With this test in place, we might've caught #8515 and #8256 earlier

@randall randall added this to the 10.0.0 milestone Nov 19, 2021
@randall randall self-assigned this Nov 19, 2021
@randall randall requested a review from bryancall as a code owner November 19, 2021 23:33
@bneradt
Copy link
Contributor

bneradt commented Nov 19, 2021

[approve ci]

tr.Processes.Default.StartBefore(server)
tr.StillRunningAfter = ts
tr.StillRunningAfter = server
tr.Processes.Default.Command = f"curl -q -s -v -k --resolve '{sni_domain}:{ts.Variables.ssl_port}:127.0.0.1' https://{sni_domain}:{ts.Variables.ssl_port}"
Copy link
Contributor

Choose a reason for hiding this comment

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

What would you think about adding a comment here about why -k is needed for our future selves. I presume its something about a self signed cert / chain doesn't validate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Almost all the tests that use curl to do https use -k or --insecure and the ones that don't, specify --cacert.

It's all copy paste!

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

I have a few minor comments, but overall this looks good. Thanks for adding coverage for this!

tr2.Processes.Default.Env = ts.Env
tr2.Processes.Default.ReturnCode = 0

ts.Disk.diags_log.Content = Testers.ContainsExpression('ERROR: ', 'ERROR')
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be added as a TestRun (tr2reload) file like you did for the multicert_config above? I think that way it will check during the execution of the reload and not globally after all the tests complete.

@randall randall force-pushed the autest_cert_loading branch 2 times, most recently from 46f6cbf to a59067e Compare November 23, 2021 03:02
With this test in place, we might've caught apache#8515 and apache#8256 earlier
@randall randall force-pushed the autest_cert_loading branch from a59067e to 3bdc3ea Compare November 23, 2021 03:09
Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Looks good!

@randall randall merged commit 7bc025c into apache:master Nov 23, 2021
zwoop pushed a commit that referenced this pull request Dec 1, 2021
With this test in place, we might've caught #8515 and #8256 earlier

(cherry picked from commit 7bc025c)
@zwoop
Copy link
Contributor

zwoop commented Dec 1, 2021

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Dec 1, 2021
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
* asf/9.2.x:
  Updated ChangeLog
  remove unused function (apache#8525)
  Adds autest to test error cases loading ssl_multicert.config (apache#8532)
  Set an appropriate callback function for OpenSSL3 (apache#8524)
  Eliminate unused code in LogObject. (apache#8541)
  Enable conf_remap_float test and remove special case for local config file (apache#8540)
  ssl_verify_test: clang-analyzer fix to account for nul sni_name (apache#8462)
  Updated ChangeLog
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Jul 26, 2022
…8532)

With this test in place, we might've caught apache#8515 and apache#8256 earlier

(cherry picked from commit 7bc025c)
@randall randall deleted the autest_cert_loading branch October 11, 2022 22:03
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