fix(certificate): failed to create certificate

duanhongyi · duanhongyi · commit 6d75b6d9b006 · 2024-03-08T16:25:45.000+08:00
diff --git a/rootfs/api/models/app.py b/rootfs/api/models/app.py
@@ -26,7 +26,8 @@
 from api.exceptions import AlreadyExists, DryccException, ServiceUnavailable
 from api.utils import generate_app_name, apply_tasks, unit_to_bytes, unit_to_millicpu
 from scheduler import KubeHTTPException, KubeException
-from .gateway import Gateway, Route
+from scheduler.resources.pod import DEFAULT_CONTAINER_PORT
+from .gateway import Gateway, Route, DEFAULT_HTTP_PORT, DEFAULT_HTTPS_PORT
 from .config import Config
 from .service import Service
 from .release import Release
@@ -37,6 +38,7 @@
 
 User = get_user_model()
 logger = logging.getLogger(__name__)
+DEFAULT_PROCFILE_TYPE = "web"
 
 
 # http://kubernetes.io/v1.1/docs/design/identifiers.html
@@ -297,7 +299,7 @@ def deploy(self, release, force_deploy=False, rollback_on_failure=True):  # noqa
                     structure = self.structure.copy()
                     # zero out canonical pod counts
                     for proctype in structure.keys():
-                        if proctype == "web":
+                        if proctype == DEFAULT_PROCFILE_TYPE:
                             structure[proctype] = 0
                     # update with the default process type.
                     structure.update(self._default_structure(release))
@@ -371,15 +373,15 @@ def deploy(self, release, force_deploy=False, rollback_on_failure=True):  # noqa
             self.log(err, logging.ERROR)
             raise ServiceUnavailable(err) from e
         for procfile_type, value in deploys.items():
-            if procfile_type == "web":  # http
-                target_port = int(value.get('envs', {}).get('PORT', 5000))
-                self._create_default_ingress(procfile_type, target_port)
+            if procfile_type == DEFAULT_PROCFILE_TYPE:  # http
+                target_port = int(value.get('envs', {}).get('PORT', DEFAULT_CONTAINER_PORT))
+                self._create_default_ingress(target_port)
             service = self.service_set.filter(procfile_type=procfile_type).first()
             if not service:
                 continue
             if prev_release and prev_release.build:
                 continue
-            if procfile_type == "web":
+            if procfile_type == DEFAULT_PROCFILE_TYPE:
                 self._verify_http_health(service, **deploys[procfile_type])
             else:
                 self._verify_tcp_health(service, **deploys[procfile_type])
@@ -816,35 +818,36 @@ def _set_default_config(self):
         config.memory = new_memory
         config.save()
 
-    def _create_default_ingress(self, procfile_type, target_port):
-        port = 80
+    def _create_default_ingress(self, target_port):
         # create default service
         try:
-            service = self.service_set.filter(procfile_type=procfile_type).latest()
+            service = self.service_set.filter(procfile_type=DEFAULT_PROCFILE_TYPE).latest()
         except Service.DoesNotExist:
-            service = Service(owner=self.owner, app=self, procfile_type=procfile_type)
-            service.add_port(port, "TCP", target_port)
+            service = Service(owner=self.owner, app=self, procfile_type=DEFAULT_PROCFILE_TYPE)
+            service.add_port(DEFAULT_HTTP_PORT, "TCP", target_port)
             service.save()
         else:
-            if service.update_port(port, "TCP", target_port):
+            if service.update_port(DEFAULT_HTTP_PORT, "TCP", target_port):
                 service.save()
         # create default gateway
         try:
             gateway = self.gateway_set.filter(name=self.id).latest()
         except Gateway.DoesNotExist:
             gateway = Gateway(app=self, owner=self.owner, name=self.id)
-            added, msg = gateway.add(port, "HTTP")
-            if not added:
-                raise DryccException(msg)
+        modified = gateway.add(DEFAULT_HTTP_PORT, "HTTP")
+        if self.tls_set.latest().certs_auto_enabled or self.domain_set.filter(
+                models.Q(certificate__isnull=False)).exists():
+            modified = gateway.add(DEFAULT_HTTPS_PORT, "HTTPS") if not modified else True
+        if modified:
             gateway.save()
         # create default route
         try:
             self.route_set.filter(name=self.id).latest()
         except Route.DoesNotExist:
             route = Route(app=self, owner=self.owner, kind="HTTPRoute", name=self.id,
-                          port=port, procfile_type=service.procfile_type)
+                          port=DEFAULT_HTTP_PORT, procfile_type=service.procfile_type)
             route.rules = route.default_rules
-            attached, msg = route.attach(gateway.name, port)
+            attached, msg = route.attach(gateway.name, DEFAULT_HTTP_PORT)
             if not attached:
                 raise DryccException(msg)
             route.save()
@@ -943,11 +946,11 @@ def _check_deployment_in_progress(self, deploys, release, force_deploy=False):
     def _default_structure(release):
         """Scale to default structure based on release type"""
         if release.build.sha and not release.build.dockerfile and \
-                (release.build.procfile and 'web' not in release.build.procfile):
+                (release.build.procfile and DEFAULT_PROCFILE_TYPE not in release.build.procfile):
             structure = {}
         # default to heroku workflow
         else:
-            structure = {'web': 1}
+            structure = {DEFAULT_PROCFILE_TYPE: 1}
         return structure
 
     def _scheduler_filter(self, **kwargs):
@@ -1119,7 +1122,8 @@ def _gather_app_settings(self, release, app_settings, process_type, replicas, vo
 
         # only web is routable
         # https://www.drycc.cc/applications/managing-app-processes/#default-process-types
-        routable = True if process_type == 'web' and app_settings.routable else False
+        routable = True if (
+            process_type == DEFAULT_PROCFILE_TYPE and app_settings.routable) else False
 
         healthcheck = config.healthcheck.get(process_type, {})
         volumes, volume_mounts = self._get_volumes_and_mounts(process_type, volumes)
diff --git a/rootfs/api/models/gateway.py b/rootfs/api/models/gateway.py
@@ -12,6 +12,8 @@
 
 User = get_user_model()
 logger = logging.getLogger(__name__)
+DEFAULT_HTTP_PORT = 80
+DEFAULT_HTTPS_PORT = 443
 
 
 class Gateway(AuditedModel):
@@ -69,8 +71,10 @@ def listeners(self):
             port, protocol = item["port"], item["protocol"]
             if item["protocol"] in ("TLS", "HTTPS"):
                 for domain in domains:
-                    secret_name = (f"{self.app.id}-auto-tls" if
-                                   auto_tls else domain.certificate.name)
+                    secret_name = f"{self.app.id}-auto-tls" if auto_tls else (
+                        domain.certificate.name if domain.certificate else None)
+                    if secret_name is None:
+                        continue
                     listeners.append({
                         "allowedRoutes": {"namespaces": {"from": "All"}},
                         "name": self._get_listener_name(port, protocol, domains.index(domain)),
@@ -316,7 +320,9 @@ def _https_enforced_to_k8s(self, parent_refs):
         rules = {
             "filters": [{
                 "type": "RequestRedirect",
-                "requestRedirect": {"port": 443, "scheme": "https", "statusCode": 301}
+                "requestRedirect": {
+                    "port": DEFAULT_HTTPS_PORT, "scheme": "https", "statusCode": 301
+                }
             }]
         }
         try:
@@ -356,7 +362,7 @@ def _get_all_parent_refs(self):
                         "name": gateway_name,
                         "sectionName": listener["name"],
                     }
-                    if listener["protocol"] == "HTTP" and listener["port"] == 80:
+                    if listener["protocol"] == "HTTP" and listener["port"] == DEFAULT_HTTP_PORT:
                         http_parent_refs.append(parent_ref)
                     else:
                         parent_refs.append(parent_ref)
diff --git a/rootfs/api/models/release.py b/rootfs/api/models/release.py
@@ -6,6 +6,7 @@
 from api.utils import dict_diff
 from api.exceptions import DryccException, AlreadyExists
 from scheduler import KubeHTTPException
+from scheduler.resources.pod import DEFAULT_CONTAINER_PORT
 from .base import UuidAuditedModel
 
 User = get_user_model()
@@ -68,8 +69,9 @@ def get_port(self):
             creds = self.get_registry_auth()
 
             if self.build.type == "buildpack":
-                self.app.log('buildpack type detected. Defaulting to $PORT 5000')
-                return 5000
+                self.app.log(
+                    'buildpack type detected. Defaulting to $PORT %s' % DEFAULT_CONTAINER_PORT)
+                return DEFAULT_CONTAINER_PORT
 
             # application has registry auth - $PORT is required
             if (creds is not None) or (settings.REGISTRY_LOCATION != 'on-cluster'):
@@ -85,7 +87,7 @@ def get_port(self):
                 return int(envs.get('PORT'))
 
             # If the user provides PORT
-            return int(envs.get('PORT', 5000))
+            return int(envs.get('PORT', DEFAULT_CONTAINER_PORT))
 
         except Exception as e:
             raise DryccException(str(e)) from e
diff --git a/rootfs/api/models/tls.py b/rootfs/api/models/tls.py
@@ -102,14 +102,18 @@ def refresh_certificate_to_k8s(self):
         namespace = name = self.app.id
         if self.certs_auto_enabled:
             hosts = [domain.domain for domain in self.app.domain_set.all()]
-            try:
-                data = self.scheduler().certificate.get(namespace, name).json()
-                version = data["metadata"]["resourceVersion"]
-                self.scheduler().certificate.put(namespace, name, hosts, version)
-            except KubeException:
-                logger.log(
-                    msg="certificate {} does not exist".format(namespace), level=logging.INFO)
-                self.scheduler().certificate.create(namespace, name, hosts)
+            if len(hosts) > 0:
+                response = self.scheduler().certificate.get(namespace, name)
+                if response.status_code == 200:
+                    data = response.json()
+                    version = data["metadata"]["resourceVersion"]
+                    self.scheduler().certificate.put(namespace, name, hosts, version)
+                else:
+                    logger.log(
+                        msg="certificate {} does not exist".format(namespace), level=logging.INFO)
+                    self.scheduler().certificate.create(namespace, name, hosts)
+            else:
+                self.app.log("skip creating certificate, no domain name set", logging.WARNING)
         else:
             self.scheduler().certificate.delete(namespace, name, ignore_exception=True)
 
diff --git a/rootfs/api/serializers.py b/rootfs/api/serializers.py
@@ -19,6 +19,7 @@
 from api.schemas.volumes import SCHEMA as VOLUMES_SCHEMA
 from api.schemas.autoscale import SCHEMA as AUTOSCALE_SCHEMA
 from api.schemas.healthcheck import SCHEMA as HEALTHCHECK_SCHEMA
+from scheduler.resources.pod import DEFAULT_CONTAINER_PORT
 
 
 User = get_user_model()
@@ -424,9 +425,9 @@ class ServiceSerializer(serializers.ModelSerializer):
 
     app = serializers.SlugRelatedField(slug_field='id', queryset=models.app.App.objects.all())
     owner = serializers.ReadOnlyField(source='owner.username')
-    port = serializers.IntegerField(default=5000)
-    protocol = serializers.CharField(default="TCP")
-    target_port = serializers.IntegerField(default=5000)
+    port = serializers.IntegerField(required=True)
+    protocol = serializers.CharField(required=True)
+    target_port = serializers.IntegerField(default=DEFAULT_CONTAINER_PORT)
     procfile_type = serializers.CharField(required=True)
 
     class Meta:
diff --git a/rootfs/api/signals.py b/rootfs/api/signals.py
@@ -20,7 +20,7 @@
 from api.tasks import retrieve_resource, send_measurements
 from api.models.app import App
 from api.models.service import Service
-from api.models.gateway import Gateway
+from api.models.gateway import Gateway, DEFAULT_HTTPS_PORT
 from api.models.appsettings import AppSettings
 from api.models.build import Build
 from api.models.certificate import Certificate
@@ -159,7 +159,11 @@ def tls_changed_handle(sender, instance: TLS, created=False, update_fields=None,
     if (update_fields and "certs_auto_enabled" in update_fields) or created:
         instance.refresh_certificate_to_k8s()
         for gateway in instance.app.gateway_set.all():
-            gateway.refresh_to_k8s()
+            if (instance.certs_auto_enabled and gateway.name == instance.app.id
+                    and gateway.add(DEFAULT_HTTPS_PORT, "HTTPS")[0]):
+                gateway.save()
+            else:
+                gateway.refresh_to_k8s()
         for route in instance.app.route_set.all():
             route.refresh_to_k8s()
 
diff --git a/rootfs/api/tests/test_gateway.py b/rootfs/api/tests/test_gateway.py
@@ -79,17 +79,23 @@ def test_create_gateway(self):
 
     def test_add_listener(self):
         app_id = self.create_app()
-        self.create_gateway(app_id, 'bing-gateway', 8000, "HTTP")
+        gateway_name = 'bing-gateway'
+        self.create_gateway(app_id, gateway_name, 8000, "HTTP")
         response = self.client.post(
             '/v2/apps/{}/gateways/'.format(app_id),
-            {'name': 'bing-gateway', 'port': 443, 'protocol': "HTTP"}
+            {'name': gateway_name, 'port': 443, 'protocol': "HTTP"}
         )
         self.assertEqual(response.status_code, 201)
         response = self.client.get('/v2/apps/{}/gateways/'.format(app_id))
-        results = [{
+        actual = None
+        for result in response.data["results"]:
+            if result["name"] == gateway_name:
+                actual = json.loads(json.dumps(result))
+                break
+        expect = {
             "app": app_id,
             "owner": "autotest",
-            "name": "bing-gateway",
+            "name": gateway_name,
             "listeners": [
                 {
                     "name": "tcp-8000-%s" % 0,
@@ -102,16 +108,17 @@ def test_add_listener(self):
                     "port": 443,
                     "protocol": "HTTP",
                     "allowedRoutes": {"namespaces": {"from": "All"}}
-                }],
+                }
+            ],
             "addresses": [{
                 "type": "IPAddress",
                 "value": "172.22.108.207"
             }]
-        }]
-        self.assertEqual(results, json.loads(json.dumps(response.data["results"])))
+        }
+        self.assertEqual(expect, actual)
 
     def add_tls_listener(self, name, protocol):
-        app_id = self.create_app()
+        app_id = self.create_app(name)
         domain, secret_name = self.create_tls_domain(app_id)
         response = self.client.post(
             '/v2/apps/{}/gateways/'.format(app_id),
@@ -127,27 +134,17 @@ def add_tls_listener(self, name, protocol):
             "app": app_id,
             "owner": "autotest",
             "name": name,
-            "listeners": [{
-                "tls": {
-                    "certificateRefs": [{
-                        "kind": "Secret",
-                        "name": secret_name
-                    }]
-                },
-                "name": listener_name,
-                "port": 443,
-                "hostname": domain,
-                "protocol": protocol,
-                "allowedRoutes": {
-                    "namespaces": {
-                        "from": "All"
-                    }
+            "listeners": [
+                {
+                    "tls": {"certificateRefs": [{"kind": "Secret", "name": secret_name}]},
+                    "name": listener_name,
+                    "port": 443,
+                    "hostname": domain,
+                    "protocol": protocol,
+                    "allowedRoutes": {"namespaces": {"from": "All"}}
                 }
-            }],
-            "addresses": [{
-                "type": "IPAddress",
-                "value": "172.22.108.207"
-            }]
+            ],
+            "addresses": [{"type": "IPAddress", "value": "172.22.108.207"}]
         }]
         self.assertEqual(results, json.loads(json.dumps(response.data["results"])))
         return app_id, domain, secret_name
@@ -176,7 +173,6 @@ def test_remove_tls(self):
             '{}/{}/domain/{}/'.format('/v2/certs', secret_name, domain)
         )
         self.assertEqual(response.status_code, 204)
-
         response = self.client.get('/v2/apps/{}/gateways/'.format(app_id))
         self.assertEqual(response.data["results"][0]["listeners"], [])
         return app_id
diff --git a/rootfs/api/tests/test_service.py b/rootfs/api/tests/test_service.py
@@ -157,7 +157,7 @@ def test_service_basic_ops(self):
         )
         self.assertEqual(response.status_code, 204, response.data)
         response = self.client.get('/v2/apps/{}/services'.format(app_id))
-        self.assertEqual(response.data["services"], [])
+        self.assertEqual(len(response.data["services"]), 0)
         # delete non-existing (1st again)
         response = self.client.delete(
             '/v2/apps/{}/services'.format(app_id),
diff --git a/rootfs/scheduler/resources/cert_manager.py b/rootfs/scheduler/resources/cert_manager.py
@@ -123,7 +123,7 @@ def manifest(api_version, namespace, name, hosts, version=None):
             data["metadata"]["resourceVersion"] = version
         return data
 
-    def get(self, namespace, name=None, **kwargs):
+    def get(self, namespace, name=None, ignore_exception=True, **kwargs):
         """
         Fetch a single certificate or a list of certificates
         """
@@ -134,7 +134,7 @@ def get(self, namespace, name=None, **kwargs):
             url = self.api('/namespaces/{}/certificates', namespace)
             message = 'get certificates'
         response = self.http_get(url)
-        if self.unhealthy(response.status_code):
+        if not ignore_exception and self.unhealthy(response.status_code):
             raise KubeHTTPException(response, message)
 
         return response
diff --git a/rootfs/scheduler/resources/pod.py b/rootfs/scheduler/resources/pod.py

Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ def test_service_basic_ops(self):`
`157`	`157`	`)`
`158`	`158`	`self.assertEqual(response.status_code, 204, response.data)`
`159`	`159`	`response = self.client.get('/v2/apps/{}/services'.format(app_id))`
`160`		`- self.assertEqual(response.data["services"], [])`
	`160`	`+ self.assertEqual(len(response.data["services"]), 0)`
`161`	`161`	`# delete non-existing (1st again)`
`162`	`162`	`response = self.client.delete(`
`163`	`163`	`'/v2/apps/{}/services'.format(app_id),`