Skip to content
26 changes: 24 additions & 2 deletions python/src/mas/cli/install/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from .catalogs import supportedCatalogs

from mas.cli.validators import (
ClusterIssuerValidator,
InstanceIDFormatValidator,
WorkspaceIDFormatValidator,
WorkspaceNameFormatValidator,
Expand All @@ -51,6 +52,7 @@
getStorageClasses,
getClusterVersion,
isClusterVersionInRange,
getClusterIssuers,
configureIngressForPathBasedRouting
)
from mas.devops.mas import (
Expand Down Expand Up @@ -846,7 +848,6 @@ def configDNSAndCerts(self):
"Unless you see an error during the ocp-verify stage indicating that the secret can not be determined you do not need to set this and can leave the response empty"
])
self.promptForString("Cluster ingress certificate secret name", "ocp_ingress_tls_secret_name", default="")

self.printH1("Configure Domain & Certificate Management")
configureDomainAndCertMgmt = self.yesOrNo('Configure domain & certificate management')
if configureDomainAndCertMgmt:
Expand Down Expand Up @@ -886,13 +887,34 @@ def configDNSAndCerts(self):
# Use MAS default self-signed cluster issuer with the default domain
self.setParam("dns_provider", "")
self.setParam("mas_domain", "")
self.setParam("mas_cluster_issuer", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason we don't need this set to empty here now, wouldn't we still want this set to "" in this case?

self.manualCerts = self.yesOrNo("Configure manual certificates")
self.setParam("mas_manual_cert_mgmt", self.manualCerts)
if self.getParam("mas_manual_cert_mgmt"):
self.manualCertsDir = self.promptForDir("Enter the path containing the manual certificates", mustExist=True)
else:
self.manualCertsDir = None
else:
# Configuring domain
if self.yesOrNo('Configure custom domain'):
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the current flow a bit confusing ...

Do you want to configure domain and cert management? No
Do you want to configure custom domain? Yes

Feel like most customers would have answered Yes to the first one, even if they wanted what's in the second path.

Think we should have a single path through this taking customers through the options, with a single prompt asking them to pick how they want to manage the certs:

if (Do you want to configure domain and cert management?)
    Configure custom domain

    Choose certificate management mode
    1. Certificate-Manager with DNS integration
    2. Certificate-Manager with existing ClusterIssuer
    3. Certificate-Manager with auto-generated self-signed certificate
    4. Manual certificate management

    > 

This can then be easily extended by @rawa-resul for his work to add support for Issuer as an alternative to ClusterIssuer.

self.promptForString("MAS top-level domain", "mas_domain")
else:
self.setParam("mas_domain", "")

# Configuring DNS
if self.yesOrNo("Do you want MAS to set up its own (self-signed) cluster issuer?"):
self.setParam("mas_cluster_issuer", "")
else:
self.printDescription([
"Select the ClusterIssuer to use from the list below:",
])
clusterIssuers = getClusterIssuers(self.dynamicClient)
if clusterIssuers is not None and len(clusterIssuers) > 0:
for clusterIssuer in clusterIssuers:
print_formatted_text(HTML(f"<LightSlateGrey> - {clusterIssuer.metadata.name}</LightSlateGrey>"))
self.params['mas_cluster_issuer'] = prompt(HTML('<Yellow>MAS Cluster Issuer</Yellow> '), validator=ClusterIssuerValidator(), validate_while_typing=False)
else:
print_formatted_text(HTML("<Red>No ClusterIssuers found on this cluster. MAS will use self-signed certificates.</Red>"))
self.setParam("mas_cluster_issuer", "")

@logMethodCall
def configDNSAndCertsCloudflare(self):
Expand Down
17 changes: 15 additions & 2 deletions python/src/mas/cli/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from prompt_toolkit.validation import Validator, ValidationError
from prompt_toolkit.document import Document

from mas.devops.ocp import getStorageClass
from mas.devops.ocp import getStorageClass, getClusterIssuer
from mas.devops.mas import verifyMasInstance
from mas.devops.aiservice import verifyAiServiceInstance, verifyAiServiceTenantInstance

Expand Down Expand Up @@ -241,4 +241,17 @@ def validate(self, document: Document) -> None:
if not match(r"^.{1,4}$", bucketPrefix):
raise ValidationError(message='Bucket prefix does not meet the requirement', cursor_position=len(bucketPrefix))

# Made with Bob

class ClusterIssuerValidator(Validator):
def validate(self, document):
"""
Validate that a ClusterIssuer exists on the target cluster
"""
name = document.text

dynClient = dynamic.DynamicClient(
api_client.ApiClient(configuration=config.load_kube_config())
)

if getClusterIssuer(dynClient, name) is None:
raise ValidationError(message='Specified cluster issuer is not available on this cluster', cursor_position=len(name))
Loading