Skip to content

Conversation

javuto
Copy link
Collaborator

@javuto javuto commented May 27, 2025

Adding the flag --enable-osctrld to enable the osctrld endpoints in osctrl-tls. By default, they are disabled.

@javuto javuto requested a review from Copilot May 27, 2025 21:43
@javuto javuto added osctrl-tls osctrl-tls related changes osctrld osctrld related issues labels May 27, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new --enable-osctrld flag to toggle the registration of osctrld-related endpoints in the TLS service.

  • Introduce Osctrld field in ServiceFlagParams and wire it into CLI flags
  • Append initOsctrldFlags to TLS flag initialization
  • Conditionally register osctrld handlers in osctrlService based on the flag

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/config/flags.go Added Osctrld flag, initOsctrldFlags, and hooked it into InitTLSFlags
cmd/tls/main.go Wrapped osctrld endpoint handlers behind flagParams.Osctrld with a log message
Comments suppressed due to low confidence (2)

pkg/config/flags.go:69

  • [nitpick] Consider renaming Osctrld to EnableOsctrld to clearly indicate it’s a boolean flag and align with other fields like TLSServer and DebugHTTPValues.Enabled.
Osctrld bool

cmd/tls/main.go:297

  • No tests were added to verify that osctrld endpoints are registered only when the flag is enabled. Add unit or integration tests to cover both flag-enabled and flag-disabled scenarios.
if flagParams.Osctrld {

javuto and others added 2 commits May 27, 2025 23:46
Usage string more precise and consistent

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@javuto javuto merged commit 49e44eb into main May 28, 2025
53 checks passed
@javuto javuto deleted the flag-tls-osctrld branch May 28, 2025 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
osctrl-tls osctrl-tls related changes osctrld osctrld related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant