Skip to content

Add keyring fallback hierarchy for containerized environments#387

Merged
Vad1mo merged 1 commit into
goharbor:mainfrom
qcserestipy:384a_keyring_container_env
Apr 10, 2025
Merged

Add keyring fallback hierarchy for containerized environments#387
Vad1mo merged 1 commit into
goharbor:mainfrom
qcserestipy:384a_keyring_container_env

Conversation

@qcserestipy

Copy link
Copy Markdown
Collaborator

Fixes #386

Problem

Harbor CLI fails to run in container environments due to missing system keyring services.

Solution

This PR implements a hierarchical keyring fallback system that prioritizes:

  1. Environment variables (HARBOR_ENCRYPTION_KEY) - ideal for containers/Kubernetes
  2. System keyring - best for desktop environments
  3. File-based storage - fallback when neither of the above is available

Implementation details

  • Added EnvironmentKeyring for containerized environments
  • Added validation of encryption key sizes with clear error messages
  • Improved debug and info logging for keyring selection

Usage examples

Docker container with environment key:

# Generate a secure AES-256 key
docker run --rm -e HARBOR_ENCRYPTION_KEY=$(openssl rand -base64 32) \
  goharbor/harbor-cli login https://demo.goharbor.io -u username -p password

Kubernetes usage:

# secrets.yaml
apiVersion: v1
kind: Secret
metadata:
  name: harbor-cli-secrets
type: Opaque
data:
  HARBOR_ENCRYPTION_KEY: "<base64-encoded-key>"

# deployment.yaml
spec:
  containers:
  - name: harbor-cli
    image: goharbor/harbor-cli
    envFrom:
    - secretRef:
        name: harbor-cli-secrets

Testing

Added comprehensive tests for keyring providers: Test_EncryptionWithEnvironmentKeyring

Documentation updates:

Where do they go? Into the main README.md?

…file based keyring provider. This enables containres to store the encryption key more safely as env var or as k8s secret in k8s pods

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
@qcserestipy

Copy link
Copy Markdown
Collaborator Author

@Vad1mo I added this additional implementation to give people the possibility to store they encryption private key somewhere else when using containers. The FilekeyRing provider should only be the non-production fallback option. In production people can store their private key in the container env or in K8s secrets. Where would we document such kind of code? And add some parts about it in the README.md?

@Vad1mo Vad1mo requested a review from Copilot April 10, 2025 09:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comments suppressed due to low confidence (2)

test/e2e/encryption_test.go:88

  • Consider adding a negative test case to verify the behavior when the expected environment variable is not set, ensuring that proper error handling is in place.
func Test_EncryptionWithEnvironmentKeyring(t *testing.T) {

pkg/utils/encryption.go:117

  • [nitpick] The error message in this Delete function currently states that deletion is not supported; consider elaborating on the limitation or referencing documentation to help users understand the intended behavior.
func (e *EnvironmentKeyring) Delete(service, user string) error {

@Vad1mo

Vad1mo commented Apr 10, 2025

Copy link
Copy Markdown
Member

@qcserestipy the examples are great, do you think we can get them out of the PR into general docs? e.g. manpages, so that users can find them later too..

@qcserestipy

Copy link
Copy Markdown
Collaborator Author

@Vad1mo Yes, I can add them into the docs, before merging. Do you have any preference about the place where to put them?

@Vad1mo

Vad1mo commented Apr 10, 2025

Copy link
Copy Markdown
Member

I don't have any preference, we have the doc dir: As a user I would expect to have everything in one place.

If we could have it somewhere that can be rendered into the website or similar, that would be great..

@Vad1mo Vad1mo merged commit 1d08f33 into goharbor:main Apr 10, 2025
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