Skip to content

Commit d5ffec2

Browse files
adwk67razvan
andauthored
feat: Git sync - add first class support for CAs (#1154)
* add ca-certs to gitsync set-up * updated comment * replace field with existing TlsClientDetails struct * apply patch to use TlsClientDetailsWithSecureDefaults with default fo webPki * set http.sslverify to false for tls: null * changelog * add scheme checks and only set verify when tls/server explicitly set to none * remove print statements * remove http scheme check * bump rustls-webpki * Update crates/stackable-operator/src/crd/git_sync/v1alpha2_impl.rs Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> --------- Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com>
1 parent 380e4fb commit d5ffec2

File tree

5 files changed

+580
-5
lines changed

5 files changed

+580
-5
lines changed

crates/stackable-operator/CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@ All notable changes to this project will be documented in this file.
66

77
### Added
88

9-
- Implement `Deref` for `kvp::Key` to be more ergonomic to use ([#1182]).
9+
- Git sync: add support for CAs ([#1154]).
1010
- Add support for specifying a `clientAuthenticationMethod` for OIDC ([#1178]).
1111
This was originally done in [#1158] and had been reverted in [#1170].
12+
- Implement `Deref` for `kvp::Key` to be more ergonomic to use ([#1182]).
1213

1314
### Removed
1415

1516
- BREAKING: Remove unused `add_prefix`, `try_add_prefix`, `set_name`, and `try_set_name` associated
1617
functions from `kvp::Key` to disallow mutable access to inner values ([#1182]).
1718

19+
[#1154]: https://github.com/stackabletech/operator-rs/pull/1154
1820
[#1178]: https://github.com/stackabletech/operator-rs/pull/1178
1921
[#1182]: https://github.com/stackabletech/operator-rs/pull/1182
2022

crates/stackable-operator/crds/DummyCluster.yaml

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,56 @@ spec:
152152
description: 'The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator` or `ssh://git@github.com:stackable-airflow/dags.git`.'
153153
format: uri
154154
type: string
155+
tls:
156+
default:
157+
verification:
158+
server:
159+
caCert:
160+
webPki: {}
161+
description: Configure a TLS connection. If not specified it will default to webPki validation.
162+
nullable: true
163+
properties:
164+
verification:
165+
description: The verification method used to verify the certificates of the server and/or the client.
166+
oneOf:
167+
- required:
168+
- none
169+
- required:
170+
- server
171+
properties:
172+
none:
173+
description: Use TLS but don't verify certificates.
174+
type: object
175+
server:
176+
description: Use TLS and a CA certificate to verify the server.
177+
properties:
178+
caCert:
179+
description: CA cert to verify the server.
180+
oneOf:
181+
- required:
182+
- webPki
183+
- required:
184+
- secretClass
185+
properties:
186+
secretClass:
187+
description: |-
188+
Name of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) which will provide the CA certificate.
189+
Note that a SecretClass does not need to have a key but can also work with just a CA certificate,
190+
so if you got provided with a CA cert but don't have access to the key you can still use this method.
191+
type: string
192+
webPki:
193+
description: |-
194+
Use TLS and the CA certificates trusted by the common web browsers to verify the server.
195+
This can be useful when you e.g. use public AWS S3 or other public available services.
196+
type: object
197+
type: object
198+
required:
199+
- caCert
200+
type: object
201+
type: object
202+
required:
203+
- verification
204+
type: object
155205
wait:
156206
default: 20s
157207
description: |-

crates/stackable-operator/src/commons/tls_verification.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub enum TlsClientDetailsError {
2626
},
2727
}
2828

29+
#[repr(transparent)]
2930
#[derive(
3031
Clone, Debug, Deserialize, Eq, Hash, JsonSchema, Ord, PartialEq, PartialOrd, Serialize,
3132
)]
@@ -35,6 +36,40 @@ pub struct TlsClientDetails {
3536
pub tls: Option<Tls>,
3637
}
3738

39+
#[repr(transparent)]
40+
#[derive(
41+
Clone, Debug, Deserialize, Eq, Hash, JsonSchema, Ord, PartialEq, PartialOrd, Serialize,
42+
)]
43+
#[serde(rename_all = "camelCase")]
44+
pub struct TlsClientDetailsWithSecureDefaults {
45+
/// Configure a TLS connection. If not specified it will default to webPki validation.
46+
#[serde(default = "default_web_pki_tls")]
47+
pub tls: Option<Tls>,
48+
}
49+
50+
impl std::ops::Deref for TlsClientDetailsWithSecureDefaults {
51+
type Target = TlsClientDetails;
52+
53+
fn deref(&self) -> &TlsClientDetails {
54+
// SAFETY: both types are `#[repr(transparent)]` over `Option<Tls>`, so they share
55+
// the same memory layout and this cast is sound.
56+
//
57+
// This cannot silently break due to struct changes: `#[repr(transparent)]` requires
58+
// exactly one non-zero-sized field, so adding a second real field to either struct
59+
// is a compile error. The only scenario that would NOT be caught at compile time is
60+
// deliberately removing `#[repr(transparent)]` from one of the two structs.
61+
unsafe { &*(self as *const Self as *const TlsClientDetails) }
62+
}
63+
}
64+
65+
fn default_web_pki_tls() -> Option<Tls> {
66+
Some(Tls {
67+
verification: TlsVerification::Server(TlsServerVerification {
68+
ca_cert: CaCert::WebPki {},
69+
}),
70+
})
71+
}
72+
3873
impl TlsClientDetails {
3974
/// This functions adds
4075
///
@@ -165,3 +200,51 @@ pub enum CaCert {
165200
/// so if you got provided with a CA cert but don't have access to the key you can still use this method.
166201
SecretClass(String),
167202
}
203+
204+
#[cfg(test)]
205+
mod tests {
206+
use super::*;
207+
use crate::utils::yaml_from_str_singleton_map;
208+
209+
#[test]
210+
fn tls_client_details_with_secure_defaults_deserialization() {
211+
// No tls key at all → WebPki default kicks in
212+
let parsed: TlsClientDetailsWithSecureDefaults =
213+
yaml_from_str_singleton_map("{}").expect("failed to deserialize empty input");
214+
assert_eq!(parsed.tls, default_web_pki_tls());
215+
216+
// Explicit null → opt out of TLS entirely
217+
let parsed: TlsClientDetailsWithSecureDefaults =
218+
yaml_from_str_singleton_map("tls: null").expect("failed to deserialize tls: null");
219+
assert_eq!(parsed.tls, None);
220+
221+
// Explicit SecretClass value is preserved as-is
222+
let parsed: TlsClientDetailsWithSecureDefaults = yaml_from_str_singleton_map(
223+
"tls:
224+
verification:
225+
server:
226+
caCert:
227+
secretClass: my-ca",
228+
)
229+
.expect("failed to deserialize secretClass");
230+
assert_eq!(
231+
parsed.tls,
232+
Some(Tls {
233+
verification: TlsVerification::Server(TlsServerVerification {
234+
ca_cert: CaCert::SecretClass("my-ca".to_owned()),
235+
}),
236+
})
237+
);
238+
}
239+
240+
#[test]
241+
#[allow(clippy::explicit_auto_deref)]
242+
fn tls_client_details_with_secure_defaults_deref() {
243+
let secure: TlsClientDetailsWithSecureDefaults =
244+
yaml_from_str_singleton_map("{}").expect("failed to deserialize");
245+
246+
// Deref must not panic and must expose the same tls value
247+
let tls_client_details: &TlsClientDetails = &*secure;
248+
assert_eq!(tls_client_details.tls, secure.tls);
249+
}
250+
}

crates/stackable-operator/src/crd/git_sync/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@ use serde::{Deserialize, Serialize};
77
use stackable_shared::time::Duration;
88
use url::Url;
99

10-
use crate::{crd::git_sync::v1alpha2::Credentials, versioned::versioned};
10+
use crate::{
11+
commons::tls_verification::TlsClientDetailsWithSecureDefaults,
12+
crd::git_sync::v1alpha2::Credentials, versioned::versioned,
13+
};
1114

1215
mod v1alpha1_impl;
1316
mod v1alpha2_impl;
1417

1518
#[versioned(version(name = "v1alpha1"), version(name = "v1alpha2"))]
1619
pub mod versioned {
20+
1721
pub mod v1alpha1 {
1822
pub use v1alpha1_impl::{Error, GitSyncResources};
1923
}
@@ -68,6 +72,12 @@ pub mod versioned {
6872
downgrade_with = credentials_to_secret
6973
))]
7074
pub credentials: Option<Credentials>,
75+
76+
/// An optional field used for referencing CA certificates that will be used to verify the git server's TLS certificate by passing it to the git config option `http.sslCAInfo` passed with the gitsync command. The secret must have a key named `ca.crt` whose value is the PEM-encoded certificate bundle.
77+
/// If `http.sslCAInfo` is also set via `gitSyncConf` (the `--git-config` option) then a warning will be logged.
78+
/// If not specified no TLS will be used, defaulting to github/lab using commonly-recognised certificates.
79+
#[serde(flatten)]
80+
pub tls: TlsClientDetailsWithSecureDefaults,
7181
}
7282

7383
#[derive(strum::Display, Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]

0 commit comments

Comments
 (0)