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

Upstream Hostname into operator-rs #494

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ All notable changes to this project will be documented in this file.
- Active Directory's `samAccountName` generation can now be customized ([#454]).
- Added experimental cert-manager backend ([#482]).

### Changed

- Refactored hostname validation ([#494]).
- BREAKING: Hostname validation is now somewhat stricter.
- BREAKING: Hostname validation is now enforced in CRD.
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yep, we had some discussions around RFC 1123 and renamed it accordingly to the RFC (at least we hope ^^), before it gaines widespread adoption. We would have involved you but you where on vacation and we did not want to further delay the PR, but please feel free to veto, happy to discuss with @Techassi!


### Fixed

- Fixed Kerberos keytab provisioning reusing its credential cache ([#490]).
Expand All @@ -21,6 +27,7 @@ All notable changes to this project will be documented in this file.
[#454]: https://github.com/stackabletech/secret-operator/pull/454
[#482]: https://github.com/stackabletech/secret-operator/pull/482
[#490]: https://github.com/stackabletech/secret-operator/pull/490
[#494]: https://github.com/stackabletech/secret-operator/pull/494
[#495]: https://github.com/stackabletech/secret-operator/pull/495
[#497]: https://github.com/stackabletech/secret-operator/pull/497

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ uuid = { version = "1.10.0", features = ["v4"] }
yasna = "0.5"

[patch."https://github.com/stackabletech/operator-rs.git"]
# stackable-operator = { path = "../operator-rs" }
# stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }
3 changes: 2 additions & 1 deletion deploy/helm/secret-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ spec:
nullable: true
properties:
prefix:
default: ''
default: ""
description: A prefix to be prepended to generated samAccountNames.
type: string
totalLength:
Expand Down Expand Up @@ -244,6 +244,7 @@ spec:
type: string
realmName:
description: The name of the Kerberos realm. This should be provided by the Kerberos administrator.
pattern: ^[-.a-zA-Z0-9]+$
type: string
required:
- admin
Expand Down
14 changes: 9 additions & 5 deletions rust/operator-binary/src/backend/kerberos_keytab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ use stackable_krb5_provision_keytab::{
self as provision,
provision_keytab,
};
use stackable_operator::{k8s_openapi::api::core::v1::Secret, kube::runtime::reflector::ObjectRef};
use stackable_operator::{
commons::networking::{HostName, KerberosRealmName},
k8s_openapi::api::core::v1::Secret,
kube::runtime::reflector::ObjectRef,
};
use stackable_secret_operator_crd_utils::SecretReference;
use tempfile::tempdir;
use tokio::{
Expand All @@ -15,8 +19,8 @@ use tokio::{

use crate::{
crd::{
ActiveDirectorySamAccountNameRules, Hostname, InvalidKerberosPrincipal,
KerberosKeytabBackendAdmin, KerberosPrincipal,
ActiveDirectorySamAccountNameRules, InvalidKerberosPrincipal, KerberosKeytabBackendAdmin,
KerberosPrincipal,
},
format::{well_known, SecretData, WellKnownSecretData},
utils::Unloggable,
Expand Down Expand Up @@ -82,8 +86,8 @@ impl SecretBackendError for Error {

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

Expand Down
52 changes: 5 additions & 47 deletions rust/operator-binary/src/crd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{fmt::Display, ops::Deref};
use serde::{Deserialize, Serialize};
use snafu::Snafu;
use stackable_operator::{
commons::networking::{HostName, KerberosRealmName},
kube::CustomResource,
schemars::{self, JsonSchema},
time::Duration,
Expand Down Expand Up @@ -175,11 +176,11 @@ pub enum CertManagerIssuerKind {
#[serde(rename_all = "camelCase")]
pub struct KerberosKeytabBackend {
/// The name of the Kerberos realm. This should be provided by the Kerberos administrator.
pub realm_name: Hostname,
pub realm_name: KerberosRealmName,

/// The hostname of the Kerberos Key Distribution Center (KDC).
/// This should be provided by the Kerberos administrator.
pub kdc: Hostname,
pub kdc: HostName,

/// Kerberos admin configuration settings.
pub admin: KerberosKeytabBackendAdmin,
Expand All @@ -200,15 +201,15 @@ pub enum KerberosKeytabBackendAdmin {
Mit {
/// The hostname of the Kerberos Admin Server.
/// This should be provided by the Kerberos administrator.
kadmin_server: Hostname,
kadmin_server: HostName,
},

/// Credentials should be provisioned in a Microsoft Active Directory domain.
#[serde(rename_all = "camelCase")]
ActiveDirectory {
/// An AD LDAP server, such as the AD Domain Controller.
/// This must match the server’s FQDN, or GSSAPI authentication will fail.
ldap_server: Hostname,
ldap_server: HostName,

/// Reference (name and namespace) to a Kubernetes Secret object containing
/// the TLS CA (in `ca.crt`) that the LDAP server’s certificate should be authenticated against.
Expand Down Expand Up @@ -255,49 +256,6 @@ impl ActiveDirectorySamAccountNameRules {
}
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(try_from = "String", into = "String")]
pub struct Hostname(String);
#[derive(Debug, Snafu)]
#[snafu(module)]
pub enum InvalidHostname {
#[snafu(display("hostname contains illegal characters (allowed: alphanumeric, -, and .)"))]
IllegalCharacter,

#[snafu(display("hostname may not start with a dash"))]
StartWithDash,
}
impl TryFrom<String> for Hostname {
type Error = InvalidHostname;

fn try_from(value: String) -> Result<Self, Self::Error> {
if value.starts_with('-') {
invalid_hostname::StartWithDashSnafu.fail()
} else if value.contains(|chr: char| !chr.is_alphanumeric() && chr != '.' && chr != '-') {
invalid_hostname::IllegalCharacterSnafu.fail()
} else {
Ok(Hostname(value))
}
}
}
impl From<Hostname> for String {
fn from(value: Hostname) -> Self {
value.0
}
}
impl Display for Hostname {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.0)
}
}
impl Deref for Hostname {
type Target = str;

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

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(try_from = "String", into = "String")]
pub struct KerberosPrincipal(String);
Expand Down
Loading