Skip to content
This repository was archived by the owner on Feb 15, 2022. It is now read-only.

[CDN-1176] Add fix to delete SNI certificate with domain name #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[CDN-1176] Add fix to delete SNI certificate with domain name #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions poppy/distributed_task/taskflow/task/delete_service_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ def execute(self, project_id, service_id):
service_id
)

storage_cert_obj = service_controller.ssl_certificate_storage

kwargs = {
'project_id': project_id,
'context_dict': context_utils.get_current().to_dict(),
Expand All @@ -242,8 +244,16 @@ def execute(self, project_id, service_id):
domain.protocol == 'https' and
domain.certificate in ['san', 'sni']
):
cert_obj = storage_cert_obj.get_certs_by_domain(
domain.domain,
cert_type=domain.certificate
)
kwargs["domain_name"] = domain.domain
kwargs["cert_type"] = domain.certificate
if cert_obj:
kwargs["cert_obj_json"] = json.dumps(cert_obj.to_dict())
else:
kwargs["cert_obj_json"] = json.dumps({})
LOG.info(
"Delete service submit task {0} cert delete "
"domain {1}.".format(
Expand Down
23 changes: 17 additions & 6 deletions poppy/distributed_task/taskflow/task/delete_ssl_certificate_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import json

from oslo_config import cfg
from oslo_log import log
Expand All @@ -30,11 +31,17 @@ class DeleteProviderSSLCertificateTask(task.Task):
default_provides = "responders"

def execute(self, providers_list, domain_name, cert_type,
project_id, flavor_id):
project_id, cert_obj_json):
service_controller = memoized_controllers.task_controllers('poppy')

cert_obj_json = json.loads(cert_obj_json)

flavor_id = cert_obj_json.get('flavor_id')
cert_details = cert_obj_json.get('cert_details')

cert_obj = ssl_certificate.SSLCertificate(flavor_id, domain_name,
cert_type, project_id)
cert_type, project_id,
cert_details)

responders = []
# try to delete all certificates from each provider
Expand Down Expand Up @@ -90,11 +97,15 @@ def execute(self, project_id, domain_name, cert_type):
self.storage_controller = self.ssl_certificate_manager.storage

try:
self.storage_controller.delete_certificate(
project_id,
domain_name,
cert_type
cert = self.storage_controller.get_certs_by_domain(
domain_name, project_id=project_id
)
if cert:
self.storage_controller.delete_certificate(
project_id,
domain_name,
cert_type
)
except ValueError as e:
LOG.exception(e)

Expand Down
2 changes: 1 addition & 1 deletion poppy/manager/default/ssl_certificate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def delete_ssl_certificate(self, project_id, domain_name, cert_type):
'domain_name': domain_name,
'cert_type': cert_type,
'cert_obj_json': json.dumps(cert_obj.to_dict()),
'providers_list_json': json.dumps(providers),
'providers_list': providers,
'context_dict': context_utils.get_current().to_dict()
}
self.distributed_task_controller.submit_task(
Expand Down
27 changes: 15 additions & 12 deletions poppy/provider/akamai/certificates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,21 @@ def delete_certificate(self, cert_obj):
)

if found is False:
# Checking for pending changes while deleting
if (cert_obj.cert_details["Akamai"]
["extra_info"]["change_url"]):
LOG.info("{0} has pending changes, skipping...".format(
cert_obj.domain_name)
)
return self.responder.ssl_certificate_deleted(
cert_obj.domain_name,
{
'status': 'failed due to pending changes',
'reason': 'Delete request for {0} failed'
.format(cert_obj.domain_name)
}
)

return self.responder.ssl_certificate_deleted(
cert_obj.domain_name,
{
Expand Down Expand Up @@ -596,18 +611,6 @@ def delete_certificate(self, cert_obj):
enrollment_id, resp.text))

resp_json = json.loads(resp.text)
# check enrollment does not have any pending changes
if len(resp_json['pendingChanges']) > 0:
LOG.info("{0} has pending changes, skipping...".format(
found_cert))
return self.responder.ssl_certificate_deleted(
cert_obj.domain_name,
{
'status': 'failed due to pending changes',
'reason': 'Delete request for {0} failed'
.format(cert_obj.domain_name)
}
)

# remove domain name from sans
resp_json['csr']['sans'].remove(cert_obj.domain_name)
Expand Down
11 changes: 7 additions & 4 deletions poppy/transport/pecan/controllers/v1/ssl_certificates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,17 @@ def post(self):
helpers.abort_with_message)
)
def delete(self, domain_name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be:

        def delete(self, domain_name):
        certificate_controller = \
            self._driver.manager.ssl_certificate_controller

        try:
            # NOTE(TheSriram): we can also enforce project_id constraints
            cert_obj = certificate_controller.get_certs_info_by_domain(
                domain_name=domain_name,
                project_id=None)
        except ValueError:
            pecan.abort(404, detail='certificate '
                                    'could not be found '
                                    'for domain : %s' %
                        domain_name)
            raise

        try:
            certificate_controller.delete_ssl_certificate(
                cert_obj.project_id, domain_name, cert_obj.cert_type
            )
        except ValueError as e:
            pecan.abort(400, detail='Delete ssl certificate failed. '
                        'Reason: %s' % str(e))

        return pecan.Response(None, 202)

    @pecan.expose('json')
    @decorators.validate(
        domain_name=rule.Rule(
            helpers.is_valid_domain_by_name(),
            helpers.abort_with_message)
    )

# For now we only support 'san' cert type
cert_type = pecan.request.GET.get('cert_type', 'san')

certificate_controller = \
self._driver.manager.ssl_certificate_controller

Copy link

Choose a reason for hiding this comment

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

existing implementation looks good. is it possible to use the existing implementation?

certificate_info_dict = json.loads(pecan.request.body.decode('utf-8'))

try:
project_id = certificate_info_dict.get('project_id')
cert_type = certificate_info_dict.get('cert_type')

certificate_controller.delete_ssl_certificate(
self.project_id, domain_name, cert_type
project_id, domain_name, cert_type
)
except ValueError as e:
pecan.abort(400, detail='Delete ssl certificate failed. '
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/distributed_task/taskflow/test_flows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1063,8 +1063,8 @@ def test_delete_ssl_certificate_normal(self):
'cert_type': "san",
'project_id': json.dumps(str(uuid.uuid4())),
'domain_name': "mytestsite.com",
'cert_obj': json.dumps(cert_obj.to_dict()),
'providers_list': json.dumps(providers),
'cert_obj_json': json.dumps(cert_obj.to_dict()),
'providers_list': providers,
'flavor_id': "premium",
'context_dict': context_utils.RequestContext().to_dict()
}
Expand Down
67 changes: 14 additions & 53 deletions tests/unit/provider/akamai/test_certificates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,13 +922,25 @@ def test_cert_create_sni_cert_pending_changes(self):
responder['Akamai']['extra_info']['action']
)

def test_cert_delete_domain_exists_on_sni_certs(self):
def test_cert_delete_sni_cert_pending_changes(self):

cert_obj = ssl_certificate.load_from_json({
"flavor_id": "flavor_id",
"domain_name": "www.abc.com",
"cert_type": "sni",
"project_id": "project_id"
"project_id": "project_id",
"cert_details": {
"Akamai": {
"cert_domain": "secured1.sni1.altcdn.com",
"extra_info": {
"status": "create_in_progress",
"change_url": "/cps/v2/enrollments/12345/changes/3418",
"created_at": "2018-06-27 06:52:46.427044",
"sni_cert": "secured1.sni1.altcdn.com",
"action": "Waiting for customer domain validation "
}
}
}
})

self.mock_sans_alternate.return_value = []
Expand All @@ -937,57 +949,6 @@ def test_cert_delete_domain_exists_on_sni_certs(self):

responder = controller.delete_certificate(cert_obj)

self.assertEqual(
'failed',
responder['Akamai']['extra_info']['status']
)
self.assertEqual(
'Domain does not exist on any certificate ',
responder['Akamai']['extra_info']['reason']
)

def test_cert_delete_sni_cert_pending_changes(self):

cert_obj = ssl_certificate.load_from_json({
"flavor_id": "flavor_id",
"domain_name": "www.abc.com",
"cert_type": "sni",
"project_id": "project_id"
})

self.mock_sans_alternate.return_value = cert_obj.domain_name

controller = certificates.CertificateController(self.driver)
controller.cert_info_storage.get_enrollment_id.return_value = 1234

controller.cps_api_client.get.return_value = mock.Mock(
status_code=200,
text=json.dumps({
"csr": {
"cn": "www.example.com",
"c": "US",
"st": "MA",
"l": "Cambridge",
"o": "Akamai",
"ou": "WebEx",
"sans": [
"example.com",
"test.example.com"
]
},
"pendingChanges": [
"/cps/v2/enrollments/234/changes/10000"
]
})
)
controller.cps_api_client.put.return_value = mock.Mock(
status_code=500,
text='INTERNAL SERVER ERROR'
)

responder = controller.delete_certificate(cert_obj)

self.assertEqual('www.abc.com', responder['Akamai']['cert_domain'])
self.assertEqual(
'failed due to pending changes',
responder['Akamai']['extra_info']['status']
Expand Down