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

Conversation

merryldmello
Copy link

CDN-1176
poppy/distributed_task/taskflow/task/delete_ssl_certificate_tasks.py
Removed flavor_id as cert_obj_json will contain all cert information.

poppy/provider/akamai/certificates.py b/poppy/provider/akamai/certificates.py
If domain is not found on certificate, we need to check pending change in progress and skip it if found; moved changes to appropriate part of the code.

poppy/transport/pecan/controllers/v1/ssl_certificates.py
Deletion of domain was hardcoded to san, and now has been accommodated for any type of cert value by accepting the request body instead.

tests/unit/distributed_task/taskflow/test_flows.py b/tests/unit/distributed_task/taskflow/test_flows.py
Changed variables respective to changes in code.

tests/unit/provider/akamai/test_certificates.py b/tests/unit/provider/akamai/test_certificates.py
Deleted a duplicate test.
Added cert details to pending changes, as it contains change url.

Copy link

@satroutr satroutr left a comment

Choose a reason for hiding this comment

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

Commit message is not pep8 compliance

)

if found is False:
if (cert_obj.cert_details["Akamai"]
Copy link

@satroutr satroutr Jun 28, 2018

Choose a reason for hiding this comment

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

Please add a comment what we are doing here. example(check if there is no pending changes in the enrolment

Copy link

@satroutr satroutr left a comment

Choose a reason for hiding this comment

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

we removed the testcase for test_cert_delete_domain_exists_on_sni_certs
dont we need them anymore


certificate_controller = \
self._driver.manager.ssl_certificate_controller

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?

The endpoint to delete domain from SNI certificate
was not supported for sni, this fix adds the support.

poppy/distributed_task/taskflow/task/delete_ssl_certificate_tasks.py
Removed flavor_id as cert_obj_json will contain all cert information.

poppy/provider/akamai/certificates.py
If domain is not found on certificate,
we need to check pending change in progress and skip it if found;
moved changes to appropriate part of the code.

poppy/transport/pecan/controllers/v1/ssl_certificates.py
Deletion of domain was hardcoded to san,
and now has been accommodated for any type of cert value
by accepting the request body instead.

tests/unit/distributed_task/taskflow/test_flows.py
Changed variables respective to changes in code.

tests/unit/provider/akamai/test_certificates.py
Deleted a duplicate test.
Added cert details to pending changes, as it contains change url.
helpers.is_valid_domain_by_name(),
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)
    )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants