fix: Set rest server http and ui ports correctly

stackabletech · Jan 12, 2024 · f922e50 · f922e50
1 parent 5485d0e
commit f922e50
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 19 deletions.
diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs
@@ -55,20 +55,25 @@ pub const HBASE_UNSAFE_REGIONSERVER_HOSTNAME_DISABLE_MASTER_REVERSEDNS: &str =
 pub const HBASE_HEAPSIZE: &str = "HBASE_HEAPSIZE";
 pub const HBASE_ROOT_DIR_DEFAULT: &str = "/hbase";
 
-pub const HBASE_UI_PORT_NAME_HTTP: &str = "ui";
+pub const HBASE_UI_PORT_NAME_HTTP: &str = "ui-http";
 pub const HBASE_UI_PORT_NAME_HTTPS: &str = "ui-https";
+pub const HBASE_REST_PORT_NAME_HTTP: &str = "rest-http";
+pub const HBASE_REST_PORT_NAME_HTTPS: &str = "rest-https";
 pub const METRICS_PORT_NAME: &str = "metrics";
 
-// TODO: Find sane port numbers for https
 pub const HBASE_MASTER_PORT: u16 = 16000;
+// HBase always uses 16010, regardless of http or https. As most products use different ports for http and https, we
+// stick to that to be consistent within the SDP.
 pub const HBASE_MASTER_UI_PORT_HTTP: u16 = 16010;
 pub const HBASE_MASTER_UI_PORT_HTTPS: u16 = 16011;
 pub const HBASE_REGIONSERVER_PORT: u16 = 16020;
 pub const HBASE_REGIONSERVER_UI_PORT_HTTP: u16 = 16030;
 pub const HBASE_REGIONSERVER_UI_PORT_HTTPS: u16 = 16031;
-// TODO: Think about https
-pub const HBASE_REST_PORT: u16 = 8080;
-pub const METRICS_PORT: u16 = 8081;
+pub const HBASE_REST_PORT_HTTP: u16 = 8080;
+pub const HBASE_REST_PORT_HTTPS: u16 = 8081;
+pub const HBASE_REST_UI_PORT_HTTP: u16 = 8085;
+pub const HBASE_REST_UI_PORT_HTTPS: u16 = 8086;
+pub const METRICS_PORT: u16 = 9100;
 
 pub const JVM_HEAP_FACTOR: f32 = 0.8;
 
@@ -623,9 +628,23 @@ impl HbaseCluster {
                 },
                 (METRICS_PORT_NAME.to_string(), METRICS_PORT),
             ],
-            // TODO: Respect HTTPS settings
             HbaseRole::RestServer => vec![
-                ("rest".to_string(), HBASE_REST_PORT),
+                if self.has_https_enabled() {
+                    (
+                        HBASE_REST_PORT_NAME_HTTPS.to_string(),
+                        HBASE_REST_PORT_HTTPS,
+                    )
+                } else {
+                    (HBASE_REST_PORT_NAME_HTTP.to_string(), HBASE_REST_PORT_HTTP)
+                },
+                if self.has_https_enabled() {
+                    (
+                        HBASE_UI_PORT_NAME_HTTPS.to_string(),
+                        HBASE_REST_UI_PORT_HTTPS,
+                    )
+                } else {
+                    (HBASE_UI_PORT_NAME_HTTP.to_string(), HBASE_REST_UI_PORT_HTTP)
+                },
                 (METRICS_PORT_NAME.to_string(), METRICS_PORT),
             ],
         }

diff --git a/rust/operator-binary/src/hbase_controller.rs b/rust/operator-binary/src/hbase_controller.rs
@@ -755,18 +755,18 @@ fn build_rolegroup_statefulset(
             ..Probe::default()
         },
         HbaseRole::RestServer => Probe {
-            // We cant use this, as it returns a 401 in case kerberos is enabled.
-            // http_get: Some(HTTPGetAction {
-            //     port: IntOrString::String("rest".to_string()),
-            //     scheme: Some(if hbase.has_https_enabled() {
-            //         "HTTPS".to_string()
-            //     } else {
-            //         "HTTP".to_string()
-            //     }),
-            //     ..HTTPGetAction::default()
-            // }),
+            // We cant use HTTPGetAction, as it returns a 401 in case kerberos is enabled, and there is currently no way
+            // to tell Kubernetes an 401 is healthy. As an alternative we run curl ourselves and check the http status
+            // code there.
             tcp_socket: Some(TCPSocketAction {
-                port: IntOrString::String("rest".to_string()),
+                port: IntOrString::String(
+                    if hbase.has_https_enabled() {
+                        "rest-https"
+                    } else {
+                        "rest"
+                    }
+                    .to_string(),
+                ),
                 ..TCPSocketAction::default()
             }),
             ..Probe::default()

diff --git a/rust/operator-binary/src/kerberos.rs b/rust/operator-binary/src/kerberos.rs
@@ -4,7 +4,8 @@ use indoc::formatdoc;
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_hbase_crd::{
     HbaseCluster, HbaseRole, CONFIG_DIR_NAME, HBASE_MASTER_UI_PORT_HTTPS,
-    HBASE_REGIONSERVER_UI_PORT_HTTPS, TLS_STORE_DIR, TLS_STORE_PASSWORD, TLS_STORE_VOLUME_NAME,
+    HBASE_REGIONSERVER_UI_PORT_HTTPS, HBASE_REST_PORT_HTTPS, HBASE_REST_UI_PORT_HTTPS,
+    TLS_STORE_DIR, TLS_STORE_PASSWORD, TLS_STORE_VOLUME_NAME,
 };
 use stackable_operator::{
     builder::{
@@ -121,8 +122,10 @@ pub fn kerberos_config_properties(hbase: &HbaseCluster) -> Result<BTreeMap<Strin
         // ("hbase.rest.support.proxyuser".to_string(), "true".to_string()),
 
         // Set non-default ports when https is enabled
+        ("hbase.rest.port".to_string(), HBASE_REST_PORT_HTTPS.to_string()),
         ("hbase.master.info.port".to_string(), HBASE_MASTER_UI_PORT_HTTPS.to_string()),
         ("hbase.regionserver.info.port".to_string(), HBASE_REGIONSERVER_UI_PORT_HTTPS.to_string()),
+        ("hbase.rest.info.port".to_string(), HBASE_REST_UI_PORT_HTTPS.to_string()),
     ]))
 }