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

Log when secrets are issued #413

Merged
merged 3 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Added

- The associated configuration is now logged for each issued secret ([#413]).

### Changed

- [BREAKING] The TLS CA Secret is now installed into the Namespace of the operator (typically `stackable-operators`), rather than `default` ([#397]).
Expand All @@ -19,6 +23,7 @@ All notable changes to this project will be documented in this file.

[#397]: https://github.com/stackabletech/secret-operator/pull/397
[#403]: https://github.com/stackabletech/secret-operator/pull/403
[#413]: https://github.com/stackabletech/secret-operator/pull/413
[#440]: https://github.com/stackabletech/secret-operator/pull/440

## [24.3.0] - 2024-03-20
Expand Down
27 changes: 22 additions & 5 deletions rust/operator-binary/src/backend/dynamic.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
//! Support code for runtime-configurable dynamic [`SecretBackend`]s

use std::{collections::HashSet, fmt::Display};
use std::{
collections::HashSet,
fmt::{Debug, Display},
};

use async_trait::async_trait;
use snafu::{ResultExt, Snafu};
use stackable_operator::kube::runtime::reflector::ObjectRef;

use crate::crd::{self, SecretClass};
use crate::{
crd::{self, SecretClass},
utils::Unloggable,
};

use super::{
kerberos_keytab::{self, KerberosProfile},
pod_info::{PodInfo, SchedulingPodInfo},
tls, SecretBackend, SecretBackendError, SecretVolumeSelector,
};

#[derive(Debug)]
pub struct DynError(Box<dyn SecretBackendError>);

impl Debug for DynError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Debug::fmt(&self.0, f)
}
}

impl Display for DynError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
Display::fmt(&self.0, f)
}
}
impl std::error::Error for DynError {
Expand All @@ -35,6 +46,12 @@ impl SecretBackendError for DynError {

pub struct DynamicAdapter<B>(B);

impl<B: Debug> Debug for DynamicAdapter<B> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
}
}

#[async_trait]
impl<B: SecretBackend + Send + Sync> SecretBackend for DynamicAdapter<B> {
type Error = DynError;
Expand Down Expand Up @@ -96,7 +113,7 @@ pub async fn from_class(
Ok(match class.spec.backend {
crd::SecretClassBackend::K8sSearch(crd::K8sSearchBackend { search_namespace }) => {
from(super::K8sSearch {
client: client.clone(),
client: Unloggable(client.clone()),
search_namespace,
})
}
Expand Down
6 changes: 4 additions & 2 deletions rust/operator-binary/src/backend/k8s_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use stackable_operator::{
kvp::{LabelError, LabelSelectorExt, Labels},
};

use crate::{crd::SearchNamespace, format::SecretData};
use crate::{crd::SearchNamespace, format::SecretData, utils::Unloggable};

use super::{
pod_info::{PodInfo, SchedulingPodInfo},
Expand Down Expand Up @@ -60,8 +60,10 @@ impl SecretBackendError for Error {
}
}

#[derive(Debug)]
pub struct K8sSearch {
pub client: stackable_operator::client::Client,
// Not secret per se, but isn't Debug: https://github.com/stackabletech/secret-operator/issues/411
pub client: Unloggable<stackable_operator::client::Client>,
pub search_namespace: SearchNamespace,
}

Expand Down
7 changes: 5 additions & 2 deletions rust/operator-binary/src/backend/kerberos_keytab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use tokio::{
use crate::{
crd::{Hostname, InvalidKerberosPrincipal, KerberosKeytabBackendAdmin, KerberosPrincipal},
format::{well_known, SecretData, WellKnownSecretData},
utils::Unloggable,
};

use super::{
Expand Down Expand Up @@ -72,15 +73,17 @@ impl SecretBackendError for Error {
}
}

#[derive(Debug)]
pub struct KerberosProfile {
pub realm_name: Hostname,
pub kdc: Hostname,
pub admin: KerberosKeytabBackendAdmin,
}

#[derive(Debug)]
pub struct KerberosKeytab {
profile: KerberosProfile,
admin_keytab: Vec<u8>,
admin_keytab: Unloggable<Vec<u8>>,
admin_principal: KerberosPrincipal,
}

Expand Down Expand Up @@ -110,7 +113,7 @@ impl KerberosKeytab {
.0;
Ok(Self {
profile,
admin_keytab,
admin_keytab: Unloggable(admin_keytab),
admin_principal,
})
}
Expand Down
4 changes: 2 additions & 2 deletions rust/operator-binary/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use stackable_operator::{
k8s_openapi::chrono::{DateTime, FixedOffset},
time::Duration,
};
use std::{collections::HashSet, convert::Infallible};
use std::{collections::HashSet, convert::Infallible, fmt::Debug};

pub use k8s_search::K8sSearch;
pub use kerberos_keytab::KerberosKeytab;
Expand Down Expand Up @@ -228,7 +228,7 @@ impl SecretContents {
/// It gets the pod information as well as volume definition and has to
/// return any number of files.
#[async_trait]
pub trait SecretBackend: Send + Sync {
pub trait SecretBackend: Debug + Send + Sync {
type Error: SecretBackendError;

/// Provision or load secret data from the source.
Expand Down
2 changes: 2 additions & 0 deletions rust/operator-binary/src/backend/pod_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub enum FromPodError {
}

/// Validated metadata about a scheduled [`Pod`]
#[derive(Debug)]
pub struct PodInfo {
pub pod_ips: Vec<IpAddr>,
pub service_name: Option<String>,
Expand Down Expand Up @@ -187,6 +188,7 @@ impl TryFrom<(AddressType, &str)> for Address {
}

/// Validated metadata about a pod that may or may not be scheduled yet.
#[derive(Debug)]
pub struct SchedulingPodInfo {
pub namespace: String,
pub pod_name: String,
Expand Down
10 changes: 6 additions & 4 deletions rust/operator-binary/src/backend/tls/ca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use tracing::{info, info_span, warn};

use crate::{
backend::SecretBackendError,
utils::{asn1time_to_offsetdatetime, Asn1TimeParseError},
utils::{asn1time_to_offsetdatetime, Asn1TimeParseError, Unloggable},
};

/// v1 format: support a single cert/pkey pair
Expand Down Expand Up @@ -156,9 +156,10 @@ pub struct Config {
}

/// A single certificate authority certificate.
#[derive(Debug)]
pub struct CertificateAuthority {
pub certificate: X509,
pub private_key: PKey<Private>,
pub private_key: Unloggable<PKey<Private>>,
not_after: OffsetDateTime,
}

Expand Down Expand Up @@ -227,7 +228,7 @@ impl CertificateAuthority {
.context(BuildCertificateSnafu)?
.build();
Ok(Self {
private_key,
private_key: Unloggable(private_key),
certificate,
not_after,
})
Expand Down Expand Up @@ -274,12 +275,13 @@ impl CertificateAuthority {
}
})?,
certificate,
private_key,
private_key: Unloggable(private_key),
})
}
}

/// Manages multiple [`CertificateAuthorities`](`CertificateAuthority`), rotating them as needed.
#[derive(Debug)]
pub struct Manager {
certificate_authorities: Vec<CertificateAuthority>,
}
Expand Down
1 change: 1 addition & 0 deletions rust/operator-binary/src/backend/tls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ impl SecretBackendError for Error {
}
}

#[derive(Debug)]
pub struct TlsGenerate {
ca_manager: ca::Manager,
max_cert_lifetime: Duration,
Expand Down
3 changes: 3 additions & 0 deletions rust/operator-binary/src/csi_server/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use snafu::{ResultExt, Snafu};
use stackable_operator::{
builder::meta::ObjectMetaBuilder,
k8s_openapi::api::core::v1::Pod,
kube::runtime::reflector::ObjectRef,
kvp::{AnnotationError, Annotations},
};
use sys_mount::{unmount, Mount, MountFlags, UnmountFlags};
Expand Down Expand Up @@ -371,6 +372,8 @@ impl Node for SecretProvisionerNode {
let backend = backend::dynamic::from_selector(&self.client, &selector)
.await
.context(publish_error::InitBackendSnafu)?;
let pod_ref = ObjectRef::<Pod>::new(&selector.pod).within(&selector.namespace);
tracing::info!(pod = %pod_ref, ?selector, ?pod_info, ?backend, "issuing secret for Pod");
let data = backend
.get_secret_data(&selector, pod_info)
.await
Expand Down
31 changes: 30 additions & 1 deletion rust/operator-binary/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
use std::{fmt::LowerHex, os::unix::prelude::AsRawFd, path::Path};
use std::{
fmt::{Debug, LowerHex},
ops::{Deref, DerefMut},
os::unix::prelude::AsRawFd,
path::Path,
};

use futures::{pin_mut, Stream, StreamExt};
use openssl::asn1::{Asn1Time, Asn1TimeRef, TimeDiff};
Expand Down Expand Up @@ -173,6 +178,30 @@ pub fn asn1time_to_offsetdatetime(asn: &Asn1TimeRef) -> Result<OffsetDateTime, A
.context(ParseSnafu)
}

/// Wrapper for (mostly) secret values that should not be logged.
// When/if migrating to Valuable, provide a dummy implementation of Value too
pub struct Unloggable<T>(pub T);

impl<T> Debug for Unloggable<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("<redacted>")
}
}

impl<T> Deref for Unloggable<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T> DerefMut for Unloggable<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

#[cfg(test)]
mod tests {
use futures::StreamExt;
Expand Down
Loading