Skip to content

CSE-364: Extend cb logdest with TLS and connection-log flags and add update#193

Open
sfc-gh-erjones wants to merge 4 commits into
mainfrom
erjones-CSE-364
Open

CSE-364: Extend cb logdest with TLS and connection-log flags and add update#193
sfc-gh-erjones wants to merge 4 commits into
mainfrom
erjones-CSE-364

Conversation

@sfc-gh-erjones
Copy link
Copy Markdown
Collaborator

Add optional --tls-verify-disabled and --forward-connection-logs to cb logdest add, and introduce cb logdest update to change an existing destination. Update merges omitted fields and unset booleans from the current cluster listing so PUT payloads stay complete.

Extend LogDestination model and list output (TTY labels and extra pipe columns) for the new fields. Wire Client#update_log_destination and teach completion to suggest the new flags and true or false after them.

Add specs for add (including invalid boolean strings), update (merge, overrides, and not-found), list, completion, captured add payloads without mock_client, and model JSON parsing with and without the new keys. Adjust the spec factory and trim spec_helper accordingly.

Note by Erik: This was written under my direction by the robot in Cursor with the model set to Auto. While it wrote some tests as we went, once the new functionality was working I asked it to do another pass for spec/test coverage on the new functionality and it found some more issues that it fixed.

…d add update

Add optional --tls-verify-disabled and --forward-connection-logs to cb logdest add,
and introduce cb logdest update to change an existing destination. Update merges
omitted fields and unset booleans from the current cluster listing so PUT payloads
stay complete.

Extend LogDestination model and list output (TTY labels and extra pipe columns) for
the new fields. Wire Client#update_log_destination and teach completion to suggest
the new flags and true or false after them.

Add specs for add (including invalid boolean strings), update (merge, overrides,
and not-found), list, completion, captured add payloads without mock_client, and
model JSON parsing with and without the new keys. Adjust the spec factory and trim
spec_helper accordingly.

Note by Erik: This was written under my direction by the robot in Cursor with the
model set to Auto. While it wrote some tests as we went, once the new functionality
was working I asked it to do another pass for spec/test coverage on the new
functionality and it found some more issues that it fixed.
@sfc-gh-erjones sfc-gh-erjones requested a review from a team as a code owner May 15, 2026 18:40
@sfc-gh-erjones
Copy link
Copy Markdown
Collaborator Author

In addition to the specs that the AI added, I did some user testing:

# No log destinations present to start

$ ssh p.cox3r5sd45e3taszjburlrahti.staging-db.pgbridgedev.com 'cat /etc/syslog-ng/conf.d/*'
Confirm user presence for key ECDSA-SK SHA256:mHjP/WE7RpOAVfD+ysp7jLGXQeY8ovwb7Klx6DNmWBo
User presence confirmed
cat: '/etc/syslog-ng/conf.d/*': No such file or directory

$ scb logdest list --cluster cox3r5sd45e3taszjburlrahti
no log destinations


# New --forward-connection-logs and --tls-verify-disabled boolean options for `cb logdest add` and new `cb logdest update` command

$ scb logdest add --help
Usage:
    cb logdest add <--cluster> <--host> <--port> <--template> [--desc] [--tls-verify-disabled <true|false> (default false)] [--forward-connection-logs <true|false> (default false)]

Options:
    --cluster ID                     Choose cluster
    --desc STR                       Description
    --forward-connection-logs BOOL   Whether to forward connection logs (true|false). Defaults to false if omitted.
    -h, --help                       Show this help
    --host HOST                      Hostname
    --port PORT                      Port number
    --template STR                   Log template
    --tls-verify-disabled BOOL       Whether TLS certificate verification is disabled (true|false). Defaults to false if omitted.

$ scb logdest update --help
Usage:
    cb logdest update <--cluster> <--logdest> [--host HOST] [--port PORT] [--template STR] [--desc STR] [--tls-verify-disabled <true|false>] [--forward-connection-logs <true|false>)]
    (Optional options keep the destination's current value when omitted.)

Options:
    --cluster ID                     Cluster ID (required)
    --desc STR                       Description (optional)
    --forward-connection-logs BOOL   Whether to forward connection logs (optional)
    -h, --help                       Show this help
    --host HOST                      Hostname (optional)
    --logdest ID                     Log destination ID (required)
    --port PORT                      Port number (optional)
    --template STR                   Log template (optional)
    --tls-verify-disabled BOOL       Whether TLS certificate verification is disabled (optional)

# Create a new logdest with everything enabled

$ scb logdest add --cluster cox3r5sd45e3taszjburlrahti \
              --desc "cb new opts test datadog" \
              --tls-verify-disabled true \
              --forward-connection-logs true \
              --host intake.logs.datadoghq.com \
              --port 10516 \
              --template 'de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n'
rward-connection-logs%20true%20%5C%0A%20%20%20%20%20%20%20%20--host%20intake.logs.datadoghq.com%20%5C%0A%20%20%20%20%20%20%20%20--port%2010516%20%5C%0A%20%20%20%20%20%20%20%20--template%20%27de18b4f38269c63861a59be7b7335813%20%3C%24%7BPRI%7D%3E1%20%24%7BISODATE%7D%20%24%7BHOST%3A--%7D%20%24%7BPROGRAM%3A--%7D%20%24%7BPID%3A--%7D%20%24%7BMSGID%3A--%7D%20%24%7BSDATA%3A--%7D%20%24Madded new log destination for cox3r5sd45e3taszjburlrahti

# list logdests

$ scb logdest list --cluster  cox3r5sd45e3taszjburlrahti
ecgvniq25nhslflmpqhkantmny
  cb new opts test datadog (intake.logs.datadoghq.com:10516)
  de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n
  TLS verify disabled: yes
  Forward connection logs: yes

# If not provided, the new options default to false

$ cb logdest destroy --cluster cox3r5sd45e3taszjburlrahti --logdest ecgvniq25nhslflmpqhkantmny
log destination destroyed

$ scb logdest list --cluster  cox3r5sd45e3taszjburlrahti
no log destinations

$ scb logdest add --cluster cox3r5sd45e3taszjburlrahti \
                    --desc "cb new opts test datadog" \
                    --host intake.logs.datadoghq.com \
                    --port 10516 \
                    --template 'de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n'
q.com%20%5C%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20--port%2010516%20%5C%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20--template%20%27de18b4f38269c63861a59be7b7335813%20%3C%24%7BPRI%7D%3Eadded new log destination for cox3r5sd45e3taszjburlrahtiM%3A--%7D%20%24%7BPID%3A--%7D%20%24%7BMSGID%3A--%7D%20%24%7BSDATA%3A--%7D%20%24MSG%5Cn%27

$ scb logdest list --cluster  cox3r5sd45e3taszjburlrahti
42fbvoirarc6hhh6unrpi7fuci
  cb new opts test datadog (intake.logs.datadoghq.com:10516)
  de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n
  TLS verify disabled: no
  Forward connection logs: no

# Let's grep the actual config to be sure that tls verification is enabled (require trust) and the connection filters are present

$ ssh p.cox3r5sd45e3taszjburlrahti.staging-db.pgbridgedev.com 'cat /etc/syslog-ng/conf.d/* | grep -E \'trusted|connection\'' 
Confirm user presence for key ECDSA-SK SHA256:mHjP/WE7RpOAVfD+ysp7jLGXQeY8ovwb7Klx6DNmWBo
User presence confirmed
            peer-verify("required-trusted")
      not message("LOG:.*connection received:") and
      not message("LOG:.*connection authenticated:") and
      not message("LOG:.*connection authorized:") and
      not message("LOG:.*disconnection: session time:")

# Excellent, now let's change them one-by-one with rechecks

$ scb logdest update --cluster  cox3r5sd45e3taszjburlrahti --logdest 42fbvoirarc6hhh6unrpi7fuci \
        --tls-verify-disabled true
updated log destination 42fbvoirarc6hhh6unrpi7fuci for cox3r5sd45e3taszjburlrahti

$ scb logdest list --cluster  cox3r5sd45e3taszjburlrahti
42fbvoirarc6hhh6unrpi7fuci
  cb new opts test datadog (intake.logs.datadoghq.com:10516)
  de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n
  TLS verify disabled: yes
  Forward connection logs: no

$ ssh p.cox3r5sd45e3taszjburlrahti.staging-db.pgbridgedev.com 'cat /etc/syslog-ng/conf.d/* | grep -E \'trusted|connection\''
Confirm user presence for key ECDSA-SK SHA256:mHjP/WE7RpOAVfD+ysp7jLGXQeY8ovwb7Klx6DNmWBo
User presence confirmed
            peer-verify("required-untrusted")
      not message("LOG:.*connection received:") and
      not message("LOG:.*connection authenticated:") and
      not message("LOG:.*connection authorized:") and
      not message("LOG:.*disconnection: session time:"

$ scb logdest update --cluster  cox3r5sd45e3taszjburlrahti --logdest 42fbvoirarc6hhh6unrpi7fuci \
              --forward-connection-logs true
updated log destination 42fbvoirarc6hhh6unrpi7fuci for cox3r5sd45e3taszjburlrahti

$ scb logdest list --cluster  cox3r5sd45e3taszjburlrahti
42fbvoirarc6hhh6unrpi7fuci
  cb new opts test datadog (intake.logs.datadoghq.com:10516)
  de18b4f38269c63861a59be7b7335813 <${PRI}>1 ${ISODATE} ${HOST:--} ${PROGRAM:--} ${PID:--} ${MSGID:--} ${SDATA:--} $MSG\n
  TLS verify disabled: yes
  Forward connection logs: yes

$ ssh p.cox3r5sd45e3taszjburlrahti.staging-db.pgbridgedev.com 'cat /etc/syslog-ng/conf.d/* | grep -E \'trusted|connection\''
Confirm user presence for key ECDSA-SK SHA256:mHjP/WE7RpOAVfD+ysp7jLGXQeY8ovwb7Klx6DNmWBo
User presence confirmed
            peer-verify("required-untrusted")

@sfc-gh-erjones
Copy link
Copy Markdown
Collaborator Author

Got CI passing after making some format/lint driven changes and adding the same codegen thread limit for the same crystal specs issue that Jesse hit in PR #191.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant