Skip to content

Conversation

@petrpinkas
Copy link
Contributor

@petrpinkas petrpinkas commented Oct 10, 2025

PR Type

Other


Description

  • Script to delete and recreate TLS certificates

  • Restarts deployments in proper dependency order

  • Automates TLS certificate restoration process


Diagram Walkthrough

flowchart LR
  A["Delete TLS secrets"] --> B["Restart Trillian DB"]
  B --> C["Restart Trillian components"]
  C --> D["Restart Redis"]
  D --> E["Restart CTlog"]
  E --> F["Display new TLS secrets"]
Loading

File Walkthrough

Relevant files
Enhancement
restoreTls.sh
TLS certificate restoration automation script                       

hack/restoreTls.sh

  • Creates bash script to delete TLS secrets for Securesign components
  • Implements ordered restart of deployments (Trillian, Redis, CTlog)
  • Adds verification step to display recreated TLS secrets
+25/-0   

@sourcery-ai
Copy link

sourcery-ai bot commented Oct 10, 2025

Reviewer's Guide

Adds a new bash script to delete existing TLS secrets in a given namespace, trigger their recreation by the operator, and sequentially restart affected deployments to ensure fresh certificates are generated.

Flow diagram for TLS certificate recreation script

flowchart TD
    A["Start script"] --> B["Delete TLS secrets"]
    B --> C["Restart Trillian components"]
    C --> D["Restart Redis"]
    D --> E["Restart CTlog"]
    E --> F["List new TLS secrets"]
    F --> G["End"]
Loading

File-Level Changes

Change Details Files
Add script to automate TLS secret recreation and deployment restarts
  • Create hack/restoreTls.sh with shebang and usage instructions
  • Delete specific TLS secrets with oc delete secret --ignore-not-found
  • Restart Trillian components in sequence via oc rollout restart
  • Restart Redis and CTlog deployments
  • List new TLS secrets with oc get secrets
grep tls

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 10, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Risky secret handling

Description: The script uses broad 'oc get secrets | grep tls' which may inadvertently print sensitive
secret names and could encourage running in the wrong namespace without explicit 'oc -n '
or 'set -e', leading to accidental deletion in the current context; add explicit namespace
scoping and safer filtering.
restoreTls.sh [7-25]

Referred Code
oc delete secret securesign-sample-rekor-redis-tls --ignore-not-found=true
oc delete secret securesign-sample-ctlog-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-logserver-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-logsigner-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-db-tls --ignore-not-found=true

echo "Restarting Trillian components ..."
oc rollout restart deployment trillian-db
oc rollout restart deployment trillian-logserver
oc rollout restart deployment trillian-logsigner

echo "Restarting Redis ..."
oc rollout restart deployment rekor-redis

echo "Restarting CTlog ..."
oc rollout restart deployment ctlog

echo "All deployments restarted. New TLS secrets:"
oc get secrets | grep tls
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `hack/restoreTls.sh:25` </location>
<code_context>
+oc rollout restart deployment ctlog
+
+echo "All deployments restarted. New TLS secrets:"
+oc get secrets | grep tls
</code_context>

<issue_to_address>
**suggestion:** Using 'grep tls' may match unintended secrets.

Consider using label selectors or a more specific pattern to ensure only TLS secrets are listed.
</issue_to_address>

### Comment 2
<location> `hack/restoreTls.sh:1-3` </location>
<code_context>
+#!/bin/bash
+
+# Delete TLS secrets (will be recreated by operator) and restart deployments in correct order.
</code_context>

<issue_to_address>
**suggestion:** Consider adding 'set -e' for safer script execution.

Placing 'set -e' after the shebang ensures the script stops on command failure, preventing unintended side effects.

```suggestion
#!/bin/bash
set -e

# Delete TLS secrets (will be recreated by operator) and restart deployments in correct order.
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 10, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Make script generic and ensure restart order

The script should be made more generic by using labels instead of hardcoded
resource names. It should also enforce the correct restart order by using oc
rollout status to wait for each deployment to complete its restart before
proceeding to the next.

Examples:

hack/restoreTls.sh [7-22]
oc delete secret securesign-sample-rekor-redis-tls --ignore-not-found=true
oc delete secret securesign-sample-ctlog-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-logserver-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-logsigner-tls --ignore-not-found=true
oc delete secret securesign-sample-trillian-db-tls --ignore-not-found=true

echo "Restarting Trillian components ..."
oc rollout restart deployment trillian-db
oc rollout restart deployment trillian-logserver
oc rollout restart deployment trillian-logsigner

 ... (clipped 6 lines)

Solution Walkthrough:

Before:

#!/bin/bash

# Delete secrets with hardcoded names
oc delete secret securesign-sample-rekor-redis-tls ...
oc delete secret securesign-sample-ctlog-tls ...
...

# Restart deployments without waiting for them to be ready
echo "Restarting Trillian components ..."
oc rollout restart deployment trillian-db
oc rollout restart deployment trillian-logserver
...

echo "Restarting Redis ..."
oc rollout restart deployment rekor-redis
...

After:

#!/bin/bash
INSTANCE_LABEL="app.kubernetes.io/instance=securesign-sample"

# Delete secrets using labels
oc delete secret -l $INSTANCE_LABEL,app.kubernetes.io/component=redis
oc delete secret -l $INSTANCE_LABEL,app.kubernetes.io/component=ctlog
...

# Restart deployments and wait for completion
echo "Restarting Trillian DB ..."
oc rollout restart deployment trillian-db
oc rollout status deployment trillian-db

echo "Restarting Trillian Log Server ..."
oc rollout restart deployment trillian-logserver
oc rollout status deployment trillian-logserver
...
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw: the script triggers restarts asynchronously without waiting, which breaks the stated goal of an ordered restart based on dependencies and could lead to failures.

High
Possible issue
Wait for deployments to finish

Wait for each deployment to complete its rollout before starting the next one.
Use oc rollout status after each oc rollout restart to prevent race conditions
between dependent components.

hack/restoreTls.sh [13-22]

 echo "Restarting Trillian components ..."
 oc rollout restart deployment trillian-db
+oc rollout status deployment/trillian-db
 oc rollout restart deployment trillian-logserver
+oc rollout status deployment/trillian-logserver
 oc rollout restart deployment trillian-logsigner
+oc rollout status deployment/trillian-logsigner
 
 echo "Restarting Redis ..."
 oc rollout restart deployment rekor-redis
+oc rollout status deployment/rekor-redis
 
 echo "Restarting CTlog ..."
 oc rollout restart deployment ctlog
+oc rollout status deployment/ctlog
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical race condition. The script's logic relies on an ordered restart, but oc rollout restart is asynchronous. Adding oc rollout status is the correct way to enforce the order and prevent potential failures.

High
Add robust error handling

Add set -e to the beginning of the script. This will ensure the script exits
immediately upon any command failure, preventing it from leaving the system in
an inconsistent state.

hack/restoreTls.sh [1]

 #!/bin/bash
 
+set -e
+
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a lack of error handling and proposes using set -e, which is a standard best practice for improving the robustness and reliability of shell scripts.

Medium
General
Improve final verification step

Improve the final verification step by explicitly getting each recreated secret.
Replace the broad oc get secrets | grep tls with specific oc get secret
commands for more accurate confirmation.

hack/restoreTls.sh [24-25]

-echo "All deployments restarted. New TLS secrets:"
-oc get secrets | grep tls
+echo "All deployments restarted. Verifying new TLS secrets:"
+oc get secret securesign-sample-rekor-redis-tls
+oc get secret securesign-sample-ctlog-tls
+oc get secret securesign-sample-trillian-logserver-tls
+oc get secret securesign-sample-trillian-logsigner-tls
+oc get secret securesign-sample-trillian-db-tls
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the final verification step is too broad and unreliable. Proposing to check for each specific secret makes the script's success validation much more robust and accurate.

Low
  • Update

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.

2 participants