Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 5514 read cabundle from kube objects - design doc #6228

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions design/20230719-cabundle-reference.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
---
title: Certificate Request CRD
authors:
- "@AppalaKarthik"
- "@sankalp-at-github"
creation-date: 2023-07-19
last-updated: 2023-07-19
#status: implementable
---

# Design: Certficate Authority Bundle Reference Using Secrets, Configmaps


<!-- toc -->
- [Problem Statement](#problem-statement)
- [Summary](#summary)
- [Motivation](#motivation)
- [Goals](#goals)
- [Non-Goals](#non-goals)
- [Proposal](#proposal)
- [Design Details](#design-details)
- [Graduation Criteria](#graduation-criteria)
- [Drawbacks](#drawbacks)
- [Alternatives](#alternatives)
<!-- /toc -->

## Problem Statement

In Issuer/ClusterIssuer resources caBundle field must be specified as a base64 encoded string.

```
spec:
venafi:
tpp:
caBundle: <B64_ENCODED_STRING>
credentialsRef:
name: tpp-token
url: https://my-server.com/vedsdk/
zone: Certificates\public
```

This becomes a configuration management issue when many issuers are there in a cluster using the same TPP endpoint, the same CA information has to be copied in all the issuers and there can be mistakes made as this would be manual.

## Summary

Ability to reference caBundle from the contents of secrets, configmap.

## Motivation
The proposed change would encourage the way caBundle is referenced in kubernetes native way.

### Goals

To be able to refer caBundle using secret.
To be able to refer caBundle using configmap.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should also specify that we want to stay up-to-date with the latest version of the resource (if the resource contents change, we want to start trusting the new CA).
TODO: check if #5387 does this properly & how it does this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I think #5387 does this correctly for secrets, ref.

if iss.Spec.Vault.CABundleSecretRef != nil {
if iss.Spec.Vault.CABundleSecretRef.Name == secret.Name {
affected = append(affected, iss)
continue
}
}
(and the same is implemented for cluster issuers).


### Non-Goals

To include reference using resources other than mentioned.

## Proposal

Enable reading of cabundle from native kubernetes objects like secrets and configmaps.

## Design Details

Add secrets and configMaps as optional fields to the spec which can be used to refer a key to find the caBundle. Below is the example for venafi issuer.

```
spec:
venafi:
tpp:
caBundle: <B64_ENCODED_STRING>
caBundleSecretRef:
name: <>
key: <>
caBundleConfigMapRef:
name: <>
key: <>
credentialsRef:
name: tpp-token
url: https://my-server.com/vedsdk/
zone: Certificates\public
```


### Graduation Criteria

Placing the `caBundleSecetRef` and `caBundleConfigMapRef` specification functionality behind a feature gate should be required.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that we have to introduce a new feature flag for this.
We also did not introduce a feature flag when we added secret refs for Vault: #5387

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we watching configmaps in cert-manager already? If not, I think it could make sense to initially add it behind a feature gate.

Placing this functionality behind a feature gate would allow the cert-manager
authors gain confidence about its correctness, and ensure there are no
regressions in the stability of controller reconciliation.

### How can this feature be enabled / disabled for an existing cert-manager installation?

By using a feature gate `--enablecabundleref`

### Will enabling / using this feature result in new API calls (i.e to Kubernetes apiserver or external services)?

This would make calls to fetch the key of secret/configmap.

## Drawbacks

Has keys for each individual reference object, instead of single key that has details of the object that can be referenced.

## Alternatives

In v2 we can implement the same using single key that can refer different type of objects
```
spec:
venafi:
tpp:
caBundleRef: # A new key could be either or with the existing `caBundle`
type: configMap # configMap | secret | bundle
name: tpp-trust-bundle
key: ca-certificates.crt
credentialsRef:
name: tpp-token
url: https://my-server.com/vedsdk/
zone: Certificates\public
```