Skip to content

Add logic to make the server.policy and server.properties settings reloadable - #9572

Merged
shinrich merged 2 commits into
masterfrom
reloadable-server-policy
Apr 3, 2023
Merged

Add logic to make the server.policy and server.properties settings reloadable#9572
shinrich merged 2 commits into
masterfrom
reloadable-server-policy

Conversation

@shinrich

Copy link
Copy Markdown
Member

Registered callback and added autest.

This closes #9563

@shinrich shinrich self-assigned this Mar 31, 2023
@shinrich
shinrich force-pushed the reloadable-server-policy branch from 40cc654 to 73f9032 Compare March 31, 2023 19:28
@shinrich
shinrich requested review from bneradt and brbzull0 March 31, 2023 21:42
@brbzull0

brbzull0 commented Apr 3, 2023

Copy link
Copy Markdown
Contributor

[approve ci cmake]

brbzull0
brbzull0 previously approved these changes Apr 3, 2023

@brbzull0 brbzull0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just a minitor comment that can be fixed in another PR probably, no biggie.

Comment thread iocore/net/SSLConfig.cc Outdated
} else if (strcmp(verify_server, "NONE") == 0) {
verifyServerProperties = YamlSNIConfig::Property::NONE;
} else {
Warning("%s is invalid for proxy.config.ssl.client.verify.server.properties. Should be one of SIGNATURE, NAME, or ALL",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this was already in the code but(not big issue) I would probably mention that NONE will be set in this case as per the docs ALL seems the default, it could be misleading and someone may think ALL will be used in this case.

Comment thread iocore/net/SSLConfig.cc Outdated
} else if (strcmp(verify_server, "ENFORCED") == 0) {
verifyServerPolicy = YamlSNIConfig::Policy::ENFORCED;
} else {
Warning("%s is invalid for proxy.config.ssl.client.verify.server.policy. Should be one of DISABLED, PERMISSIVE, or ENFORCED",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, I would mention that this is now DISABLED

@shinrich

shinrich commented Apr 3, 2023

Copy link
Copy Markdown
Member Author

@brbzull0 good points. I pushed up another commit to address.

@shinrich
shinrich force-pushed the reloadable-server-policy branch from 8dcfc2d to a385c9c Compare April 3, 2023 13:49

@brbzull0 brbzull0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@shinrich
shinrich merged commit 6d1382d into master Apr 3, 2023
@zwoop

zwoop commented Aug 28, 2023

Copy link
Copy Markdown
Contributor

@shinrich @bneradt Do we want this back ported to 9.2.x ?

@shinrich

Copy link
Copy Markdown
Member Author

Seems like a bug fix that would be good to backport. I'm not relying on this fix however. So getting it in 10.x would be good enough for me. Seems low risk in any case.

@zwoop zwoop added this to the 10.0.0 milestone Aug 29, 2023
zwoop pushed a commit that referenced this pull request Aug 29, 2023
…loadable (#9572)

(cherry picked from commit 6d1382d)

 Conflicts:
	iocore/net/P_SSLConfig.h
@zwoop

zwoop commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.3 Aug 29, 2023
@shinrich
shinrich deleted the reloadable-server-policy branch September 19, 2023 13:33
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master: (42 commits)
  Add logic to make the server.policy and server.properties settings reloadable (apache#9572)
  Add CMake to the required PR CI builds (apache#9575)
  fixup cmake build for master and add conditional for io_uring support (apache#9571)
  Cleanup: Use swoc::meta instead of ts::meta. (apache#9566)
  codeql 24: Multiplication result converted to larger type (apache#9569)
  Drop support for old quiche (apache#9561)
  QUIC: Ignore default_inactivity_timeout in favour of proxy.config.quic.no_activity_timeout_in. (apache#9564)
  Fix log format specifications (apache#9568)
  Add `current_time_epoch_ms` stat to be appended before the server version. This allows computation of stats externally based on the cache time frame. This can help alleviate issues with sliding windows between various stats programs that generate discrepencies (apache#9567)
  Define BIO macros in ink_ssl.h (apache#9557)
  combine UDPPacket and UDPPacketInternal (apache#9424)
  Update codeql.yml (apache#9560)
  Http2 to origin (apache#9366)
  coverity 1497413: Use of 32-bit time_t (apache#9556)
  Add support for multiple yaml config files for wasm plugin (apache#9483)
  Add TS_HAS_QUICHE feature variable. (apache#9547)
  mime header field parsing fix trailing quote handlling (apache#9513)
  Make magick plugin buildable with BoringSSL (apache#9554)
  QUIC: Test basic scenarios around the ts.quic.no_activity_timeout_in config. (apache#9543)
  Fix records events deps (apache#9511)
  ...
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.

proxy.config.ssl.client.verify.server.policy and proxy.config.ssl.client.verify.server.properties are not reloadable

3 participants