Skip to content

Conversation

@clement0010
Copy link
Contributor

@clement0010 clement0010 commented Oct 9, 2025

Changes

  • Add certificateAuthorityCertSecretRef on TwingateResource CRD. This enables Kubernetes Resource CRD to reference K8s secret object to update/reconcile the certificate authority cert value on Twingate via Public API.
  • Extract k8s_get_secret to utils_k8s
  • Move get_ca_cert to ResourceProxy

example_cluster_ip_gateway_service_body, "default"
)

def test_kubernetes_resource_type_annotation_with_invalid_secret_type(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests are moved to test_utils_k8s

Copy link

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

This PR adds support for referencing Kubernetes Secret objects in the TwingateResource CRD to manage certificate authority certificates, providing an alternative to inline certificate strings.

  • Add certificateAuthorityCertSecretRef field to TwingateResource CRD schema with validation
  • Update resource creation handlers to use secret references instead of inline certificates
  • Extract certificate handling utilities to shared utils_k8s module for better code organization

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
deploy/twingate-operator/crds/twingate.com.twingateresources.yaml Updates CRD schema to support certificateAuthorityCertSecretRef with oneOf validation
app/crds.py Adds secret reference support to ResourceProxy class with certificate retrieval logic
app/utils_k8s.py Extracts and consolidates Kubernetes secret handling utilities
app/handlers/handlers_services.py Updates service handler to use secret references instead of inline certificates
tests_integration/test_resource_flows.py Adds comprehensive integration test for Kubernetes resource flows with secret references
tests_integration/test_crds_resource.py Adds validation tests for new CRD schema constraints
app/tests/test_*.py Updates unit tests to use extracted utilities and test new functionality
tests_integration/utils.py Adds utility function for log message validation
app/conftest.py Moves shared test fixture to common location

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@minhtule minhtule left a comment

Choose a reason for hiding this comment

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

Fix & merge!

app/utils_k8s.py Outdated
f"Kubernetes Secret object: {tls_secret_name} type is invalid."
)

if not (ca_cert := tls_secret.data.get("ca.crt")):
Copy link
Contributor

Choose a reason for hiding this comment

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

This ca.crt is a convention in Gateway. I think it's good enough for now. Later we might want to make it customizable like ACMEIssuer.privateKeySecretRef in cert-manager.

PrivateKey is the name of a Kubernetes Secret resource that will be used to store the automatically generated ACME account private key. Optionally, a key may be specified to select a specific entry within the named Secret resource. If key is not specified, a default of tls.key will be used.

@clement0010 clement0010 force-pushed the feat/support-ca-cert-secret-ref branch from a850ad4 to 9ea0ddb Compare October 10, 2025 02:57
Copy link
Contributor

@minhtule minhtule left a comment

Choose a reason for hiding this comment

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

Fix & merge!

@clement0010 clement0010 requested a review from ekampf October 10, 2025 07:46
app/utils_k8s.py Outdated
raise


def k8s_get_secret(namespace: str, name: str) -> kubernetes.client.V1Secret | None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Other methods in this module also get the kapi: kubernetes.client.CoreV1Api | None = None kwarg and only instanciate a new CoreV1API if not given one.
Lets keep the same behavior here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, agree! I also updated the naming to k8s_read_namespaced_secret to match other methods in this module

app/utils_k8s.py Outdated
raise


def get_ca_cert(secret: kubernetes.client.V1Secret) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be here because its not purely a "k8s utility" thats general - its very specific to the CA secret of ResourceProxy - maybe it should be there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the method to ResourceProxy

@clement0010 clement0010 requested a review from ekampf October 20, 2025 13:38
Comment on lines +110 to +114
"certificateAuthorityCert": base64.b64encode(
ResourceProxy.read_certificate_authority_cert_from_secret(
secret
).encode()
).decode(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is conversion is temporary, it'll be removed in this PR

@coveralls
Copy link
Collaborator

coveralls commented Oct 20, 2025

Pull Request Test Coverage Report for Build 18672859610

Details

  • 42 of 43 (97.67%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.08%) to 96.522%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/crds.py 24 25 96.0%
Totals Coverage Status
Change from base Build 18672848609: -0.08%
Covered Lines: 1241
Relevant Lines: 1267

💛 - Coveralls

@clement0010 clement0010 force-pushed the feat/support-ca-cert-secret-ref branch from 75ba988 to e31b36e Compare October 21, 2025 02:55
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.

4 participants