Skip to content

Fail sni.yaml loading if related resources fail to load#9132

Merged
randall merged 1 commit intoapache:masterfrom
randall:sni_load_bug
Oct 13, 2022
Merged

Fail sni.yaml loading if related resources fail to load#9132
randall merged 1 commit intoapache:masterfrom
randall:sni_load_bug

Conversation

@randall
Copy link
Copy Markdown
Contributor

@randall randall commented Oct 10, 2022 •
edited
Loading

Fixes #9093

Co-authored-by: Chris McFarlen cmcfarlen@apple.com

@randall randall added the Bug label Oct 10, 2022
@randall randall self-assigned this Oct 10, 2022
@randall randall added this to the 10.0.0 milestone Oct 10, 2022
@randall
Copy link
Copy Markdown
Contributor Author

randall commented Oct 10, 2022

This should go into 9.2 and probably others, but they'll require version specific PRs due to slight source reorg/rename.

@randall randall force-pushed the sni_load_bug branch 2 times, most recently from e4965c7 to ab046d7 Compare October 10, 2022 19:13
@ywkaras
Copy link
Copy Markdown
Contributor

ywkaras commented Oct 10, 2022

[approve ci autest]

@bryancall bryancall requested a review from bneradt October 10, 2022 23:08
@ywkaras
Copy link
Copy Markdown
Contributor

ywkaras commented Oct 10, 2022

Looks like there is a hard fail for Au test tls_check_cert_selection_reload.

@randall
Copy link
Copy Markdown
Contributor Author

randall commented Oct 11, 2022

Looks like there is a hard fail for Au test tls_check_cert_selection_reload.

Thanks, I think I got it happy now.

bneradt
bneradt requested changes Oct 11, 2022
View reviewed changes
Copy link
Copy Markdown
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 to me.

Do you think we can add an autest to verify this? Alan wrote some tests for remap reload failure with the following PR, in case that's helpful:

https://github.com/apache/trafficserver/pull/8802/files

Comment thread iocore/net/SSLSNIConfig.h
Comment thread iocore/net/SSLSNIConfig.h
@randall
Copy link
Copy Markdown
Contributor Author

randall commented Oct 12, 2022

Looks good to me.

Do you think we can add an autest to verify this? Alan wrote some tests for remap reload failure with the following PR, in case that's helpful:

https://github.com/apache/trafficserver/pull/8802/files

Added (thanks to @cmcfarlen !)

bneradt
bneradt requested changes Oct 13, 2022
View reviewed changes
Copy link
Copy Markdown
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 great. Thank you for working on the test. I just have a few minor updates for it.

Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
Comment thread tests/gold_tests/tls/tls_sni_yaml_reload.test.py Outdated
@randall randall requested a review from bneradt October 13, 2022 15:21
@randall
Copy link
Copy Markdown
Contributor Author

randall commented Oct 13, 2022

oops, I didn't release you added more comments @bneradt. Sorry for the re-review request.

@randall randall force-pushed the sni_load_bug branch 6 times, most recently from 1171a98 to 0afc3c8 Compare October 13, 2022 22:23
@randall @cmcfarlen
Fail sni.yaml loading if related resources fail to load
9f3e24e 
Fixes apache#9093

Co-authored-by: Chris McFarlen <cmcfarlen@apple.com>
bneradt
bneradt approved these changes Oct 13, 2022
View reviewed changes
Copy link
Copy Markdown
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 to me.

@randall randall merged commit 215f17d into apache:master Oct 13, 2022
@randall randall deleted the sni_load_bug branch October 13, 2022 23:06
zwoop pushed a commit that referenced this pull request Oct 17, 2022
@randall @cmcfarlen @zwoop
Fail sni.yaml loading if related resources fail to load (#9132)
7eb1df7 
Fixes #9093

Co-authored-by: Chris McFarlen <cmcfarlen@apple.com>
(cherry picked from commit 215f17d)
@zwoop
Copy link
Copy Markdown
Contributor

zwoop commented Oct 17, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Oct 17, 2022
Ftywan pushed a commit to Ftywan/trafficserver that referenced this pull request Dec 21, 2022
@randall
Fail sni.yaml loading if related resources fail to load (apache#9132)
d163717 
Fixes apache#9093

Co-authored-by: Chris McFarlen <cmcfarlen@apple.com>

(cherry picked from commit 215f17d)
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
@zwoop
Merge remote-tracking branch 'asf/9.2.x' into 9.2.x
1a3e7db 
* asf/9.2.x:
  Updated ChangeLog
  Fail sni.yaml loading if related resources fail to load (apache#9132)
  fix contradicting documentation and say a bit about the resident size of a volume directory (apache#9133)
  AuTest automatic keylog file configuration (apache#9137)
  Traffic Dump: fix YAML format for CONNECT requests (apache#9139)
  Updated ChangeLog
  Remove intermediate buffer in PluginVC (apache#8698)
  Fix HTTP/2 session receive window handling for small sizes (apache#9117) (apache#9122)
  9.2: Fix s3_auth_config test output check (apache#9123)

 Conflicts:
	CHANGELOG-9.2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bneradt bneradt bneradt approved these changes

Assignees

@randall randall

Labels

Bug Configuration SSL

Projects

None yet

Milestone

9.2.0

Development

Successfully merging this pull request may close these issues.

9.2: mTLS client key load failure should revert to last known good config

5 participants

@randall @ywkaras @zwoop @bneradt @bryancall