feat: add ingress score #32
17
Cargo.lock
generated
@ -1172,6 +1172,16 @@ dependencies = [
|
||||
"percent-encoding",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "fqdn"
|
||||
version = "0.4.6"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c0f5d7f7b3eed2f771fc7f6fcb651f9560d7b0c483d75876082acb4649d266b3"
|
||||
dependencies = [
|
||||
"punycode",
|
||||
"serde",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "funty"
|
||||
version = "2.0.0"
|
||||
@ -1397,6 +1407,7 @@ dependencies = [
|
||||
"directories",
|
||||
"dockerfile_builder",
|
||||
"env_logger",
|
||||
"fqdn",
|
||||
"harmony_macros",
|
||||
"harmony_types",
|
||||
"helm-wrapper-rs",
|
||||
@ -3016,6 +3027,12 @@ dependencies = [
|
||||
"unicode-ident",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "punycode"
|
||||
version = "0.4.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e9e1dcb320d6839f6edb64f7a4a59d39b30480d4d1765b56873f7c858538a5fe"
|
||||
|
||||
[[package]]
|
||||
name = "quote"
|
||||
version = "1.0.40"
|
||||
|
||||
@ -39,3 +39,11 @@ lazy_static = "1.5.0"
|
||||
dockerfile_builder = "0.1.5"
|
||||
temp-file = "0.1.9"
|
||||
convert_case.workspace = true
|
||||
fqdn = { version = "0.4.6", features = [
|
||||
|
|
||||
"domain-label-cannot-start-or-end-with-hyphen",
|
||||
"domain-label-length-limited-to-63",
|
||||
"domain-name-without-special-chars",
|
||||
"domain-name-length-limited-to-255",
|
||||
"punycode",
|
||||
"serde",
|
||||
] }
|
||||
|
||||
@ -12,10 +12,10 @@ use super::resource::{K8sResourceInterpret, K8sResourceScore};
|
||||
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
pub struct K8sIngressScore {
|
||||
johnride
commented
I think it's now worth the time to use correct types for each k8s field types. Name should not be a string as it doesn't support all utf-8 strings, host should probably be a URL, etc. Look at the k8s spec and implement the right types. This will allow us to create macros eventually for a compile-safe DX/UX. I think it's now worth the time to use correct types for each k8s field types.
Name should not be a string as it doesn't support all utf-8 strings, host should probably be a URL, etc. Look at the k8s spec and implement the right types.
This will allow us to create macros eventually for a compile-safe DX/UX.
|
||||
pub name: String,
|
||||
pub host: String,
|
||||
pub backend_service: String,
|
||||
pub port: String,
|
||||
pub name: fqdn::FQDN,
|
||||
pub host: fqdn::FQDN,
|
||||
pub backend_service: fqdn::FQDN,
|
||||
pub port: u16,
|
||||
|
johnride
commented
should be integer https://docs.redhat.com/en/documentation/openshift_container_platform/4.15/html/network_apis/ingress-networking-k8s-io-v1#spec-rules-http-paths-backend-service-port
|
||||
pub path: Option<String>,
|
||||
pub path_type: Option<String>,
|
||||
pub namespace: Option<String>,
|
||||
|
johnride
commented
What about path, path_type and namespace? These certainly don't support any String? What about path, path_type and namespace? These certainly don't support any String?
taha
commented
Seems like validating the Fixed NS and PathType though Seems like validating the `path` is not as straightforward as the rest, as it has different validations based on the PathType and other things: https://github.com/kubernetes/ingress-nginx/issues/11176
Fixed NS and PathType though
johnride
commented
https://kubernetes.io/docs/reference/kubernetes-api/service-resources/ingress-v1/#IngressSpec
|
||||
@ -67,6 +67,6 @@ impl<T: Topology + K8sclient> Score<T> for K8sIngressScore {
|
||||
}
|
||||
|
||||
fn name(&self) -> String {
|
||||
"K8sIngressScore".to_string()
|
||||
format!("{} K8sIngressScore", self.name)
|
||||
|
johnride
commented
Not specific to this p-r but in general I think we should revisit the concept of Score names... this is not very useful in a list of multiple K8sIngressScores in the TUI for example. Not specific to this p-r but in general I think we should revisit the concept of Score names... this is not very useful in a list of multiple K8sIngressScores in the TUI for example.
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
use convert_case::{Case, Casing};
|
||||
use dockerfile_builder::instruction::{CMD, COPY, ENV, EXPOSE, FROM, RUN, WORKDIR};
|
||||
use dockerfile_builder::{Dockerfile, instruction_builder::EnvBuilder};
|
||||
use fqdn::fqdn;
|
||||
use non_blank_string_rs::NonBlankString;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
@ -134,10 +135,10 @@ impl<T: Topology + K8sclient + HelmCommand> Interpret<T> for LAMPInterpret {
|
||||
info!("LAMP deployment_score {deployment_score:?}");
|
||||
|
||||
|
johnride
commented
No, after writing this I think it's fine to impose an ingress creation here. Most use cases will want one. If/when we stumble on a legitimate use case to disable the ingress we will add the feature. But for now it breaks the idea of the LAMPScore which is to provide the simplest way to deploy a LAMP application anywhere. ~~There should maybe be an option in LampConfig to enable/disable ingress creation~~
No, after writing this I think it's fine to impose an ingress creation here. Most use cases will want one. If/when we stumble on a legitimate use case to disable the ingress we will add the feature. But for now it breaks the idea of the LAMPScore which is to provide the simplest way to deploy a LAMP application anywhere.
johnride
commented
(out of scope for this PR but worth the read here in context) Below comment related to this ADR : https://git.nationtech.io/NationTech/harmony/src/branch/master/adr/003-infrastructure-abstractions.md Thinking about the idea of using a K8sIngressScore here : A very important idea in Harmony is that we provide an opinionated infrastructure (generally K8s + Ceph + OPNSense) but the user knows nothing about it. However, the next stage is, as we already did for all the firewall related services (LoadBalancer, Router, TftpServer, etc), is to create sensibles abstractions of the infrastructure components of an application. That means that we should define a harmony "Ingress" that only has information required by harmony to deploy the concept of an ingress. Then Harmony will provide opinionated implementations of the Ingress object for the various supported Topologies. For now, this covers only k8s stuff, but at some point (probably soon), it will make sense to provide other implementations. Let's say for one of our current clients, we have to make a deployment on a windows server. Instead of using k3d, we could provide a more windows friendly implementation based on something else, which is not easily doable when we define a K8sIngressScore, but would be totally natural if it were an IngressScore that requires the Ingress capability in the associated Topology. So the signature would become : (out of scope for this PR but worth the read here in context)
Below comment related to this ADR : https://git.nationtech.io/NationTech/harmony/src/branch/master/adr/003-infrastructure-abstractions.md
Thinking about the idea of using a K8sIngressScore here :
A very important idea in Harmony is that we provide an opinionated infrastructure (generally K8s + Ceph + OPNSense) but the user knows nothing about it.
However, the next stage is, as we already did for all the firewall related services (LoadBalancer, Router, TftpServer, etc), is to create sensibles abstractions of the infrastructure components of an application.
That means that we should define a harmony "Ingress" that only has information required by harmony to deploy the concept of an ingress. Then Harmony will provide opinionated implementations of the Ingress object for the various supported Topologies.
For now, this covers only k8s stuff, but at some point (probably soon), it will make sense to provide other implementations.
Let's say for one of our current clients, we have to make a deployment on a windows server. Instead of using k3d, we could provide a more windows friendly implementation based on something else, which is not easily doable when we define a K8sIngressScore, but would be totally natural if it were an IngressScore that requires the Ingress capability in the associated Topology.
So the signature would become :
```rust
impl <T: Topology + Ingress> Score<T> for IngressScore { ... }
pub struct IngressConfig {
pub port: integer,
pub host: Hostname, // probably a harmony specific type too
// Other fields that are required in the abstraction of an ingress, nothing implementation specific
}
pub trait Ingress {
fn create(&self, config: IngressConfig) -> Result<...>
}
impl Ingress for K8sAnywhereTopology {
fn create(&self, config: IngressConfig) -> Result<...> {
K8sIngressScore::from(config); // just an idea here but it could make sense to have an easy way to generate a score that is specific to the current topology's technology from the more core Harmony abstract Scores? I feel like this would favor a cohesive, yet easy to use set of abstract/concrete scores.
}
}
```
|
||||
let lamp_ingress = K8sIngressScore {
|
||||
name: "lamp-ingress".to_string(),
|
||||
host: "test".to_string(),
|
||||
backend_service: "test".to_string(),
|
||||
port: "8080".to_string(),
|
||||
name: fqdn!("lamp-ingress"),
|
||||
host: fqdn!("test"),
|
||||
backend_service: fqdn!("test"),
|
||||
port: 8080,
|
||||
path: None,
|
||||
path_type: None,
|
||||
namespace: self.get_namespace().map(|nbs| nbs.to_string()),
|
||||
|
||||
https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names