Skip to content

Conversation

@smallc2009
Copy link
Contributor

@smallc2009 smallc2009 commented Oct 28, 2025

✨ Summary

The use of secret annotations has been discussed in several opened and closed issues. Some users require them(e.g, for ArgoCD), while others do not(they believe propagating annotations can be dangerous). As a result, the annotation has been removed in a previous commit .

In our use case, secret annotations are necessary because the Reflector depends on them to replicate secrets across namespaces.

This pull request introduces a new feature that allows the operator to optionally add custom annotations to the Kubernetes secrets it manages. This is controlled via a new --enable-annotations flag. It is disabled by default and the annotations are only added when the flag is enabled.

Feature: Optional Annotations for Managed Secrets

  • Added a new --enable-annotations flag to the command-line interface, allowing operators to specify whether managed resources should include custom annotations.
  • When enabled, the operator will propagate custom annotations on managed Secret resources.
apiVersion: apps/v1
kind: Deployment
metadata:
  name: onepassword-connect-operator
spec:
  selector:
    matchLabels:
      name: onepassword-connect
  template:
    metadata:
      labels:
        app.kubernetes.io/component: operator
        name: onepassword-connect
    spec:
      containers:
      - args:
        - --zap-log-level=debug
        - --enable-annotations
        command:
        - /manager
        env:
        - name: POLLING_INTERVAL
          value: "600"

Fix

🔗 Resolves:

#144
#178
#114

✅ Checklist

  • 🖊️ Commits are signed
  • 🧪 Tests added/updated: (See the Testing Guide for when to use each type and how to run them)
    • 🔹 Unit
    • 🔸 Integration
    • 🌐 E2E (Connect)
    • 🔑 E2E (Service Account)
  • 📚 Docs updated (if behavior changed)

🕵️ Review Notes & ⚠️ Risks

I've tested in our real development environment and testing environment. it worked as expected.

@ag-rdoucette
Copy link

This looks pretty solid.

I like the new --enable-annotations flag. It's opt-in and off by default, so it keeps things secure for people who don't need it but it still fixes those ArgoCD and Reflector integration issues.

The way it copies annotations over conditionally looks clean. It doesn't mess with the operator's logic.

I think this would be a good candidate to be approved.

@volodymyrZotov
Copy link
Contributor

Thank you for the contribution 🙌
I've added this to my list. Will review next week.

@volodymyrZotov
Copy link
Contributor

/ok-to-test sha=7b32dc8

@github-actions
Copy link
Contributor

✅ E2E tests passed.

View test run output

Copy link
Contributor

@volodymyrZotov volodymyrZotov left a comment

Choose a reason for hiding this comment

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

Great improvement! Thank you! Approved! ✅

Will merge and release beginning next week!

@volodymyrZotov volodymyrZotov merged commit ec1cde6 into 1Password:main Dec 1, 2025
5 of 6 checks passed
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.

3 participants