Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rotation toggle and filtering to table view #3646

Merged
merged 7 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 45 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,51 @@
Changelog
=========

0.10.0 - `2021-06-28`
~~~~~~~~~~~~~~~~~~~~~

This release introduces a breaking change (PR `#3646 <https://github.com/Netflix/lemur/pull/3646>`_) to the following API endpoint:

- `POST /certificates/1/update/notify <https://lemur.readthedocs.io/en/latest/developer/index.html#lemur.certificates.views.Certificates.post>`_

The endpoint is now:

- `POST /certificates/1/update/switches <https://lemur.readthedocs.io/en/latest/developer/index.html#lemur.certificates.views.Certificates.post>`_

The new endpoint honors the existing `notify` request parameter, and additionally accepts a new `rotation` parameter.
As a result of this change, the certificate table view now includes rotation switches and filtering by rotation status.


Other notable changes in this release:

- ACME:
- New celery task to prevent duplicate certificates from being autorotated
- ACME DNS-01 Challenges are supported in synchronous mode
- DNS provider check fails gracefully if not found
- Authentication:
- SSO auth now returns a newly created user during initial login
- CSRF protection is added to OAuth2.0
- Notifications:
- New reissue failed notification
- New reissue with no endpoints notification
- New revocation notification
- Plugins:
- Plugin option values are validated server-side
- Some plugin option validations updated to compile successfully server-side
- Database:
- Source and Destination deletions remove certificate associations with new confirmation dialog
- Dependency updates and conflict resolutions
- Expanded audit logs

And several smaller bugfixes and improvements.

Special thanks to all who contributed to this release, notably:

- `havron <https://github.com/havron>`_
- `tho <https://github.com/tho>`_
- `mizzy <https://github.com/mizzy>`_


0.9.0 - `2021-03-17`
~~~~~~~~~~~~~~~~~~~~

Expand Down
1 change: 1 addition & 0 deletions lemur/certificates/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ class CertificateShortOutputSchema(LemurOutputSchema):
name = fields.String()
owner = fields.Email()
notify = fields.Boolean()
rotation = fields.Boolean()
authority = fields.Nested(AuthorityNestedOutputSchema)
issuer = fields.String()
cn = fields.String()
Expand Down
12 changes: 9 additions & 3 deletions lemur/certificates/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,18 @@ def cleanup_owner_roles_notification(owner_name, kwargs):
kwargs["notifications"] = [n for n in kwargs["notifications"] if not n.label.startswith(notification_prefix)]


def update_notify(cert, notify_flag):
def update_switches(cert, notify_flag=None, rotation_flag=None):
"""
Toggle notification value which is a boolean
Toggle notification and/or rotation values which are boolean
:param notify_flag: new notify value
:param rotation_flag: new rotation value
:param cert: Certificate object to be updated
:return:
"""
cert.notify = notify_flag
if notify_flag is not None: # check for None allows value of False to continue
cert.notify = notify_flag
if rotation_flag is not None:
cert.rotation = rotation_flag
return database.update(cert)


Expand Down Expand Up @@ -561,6 +565,8 @@ def render(args):
)
elif "notify" in filt:
query = query.filter(Certificate.notify == truthiness(terms[1]))
elif "rotation" in filt:
query = query.filter(Certificate.rotation == truthiness(terms[1]))
elif "active" in filt:
query = query.filter(Certificate.active == truthiness(terms[1]))
elif "cn" in terms:
Expand Down
15 changes: 9 additions & 6 deletions lemur/certificates/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ def get(self):
parser.add_argument("owner", type=inputs.boolean, location="args")
parser.add_argument("id", type=str, location="args")
parser.add_argument("active", type=inputs.boolean, location="args")
parser.add_argument("rotation", type=inputs.boolean, location="args")
parser.add_argument(
"destinationId", type=int, dest="destination_id", location="args"
)
Expand Down Expand Up @@ -931,21 +932,22 @@ def put(self, certificate_id, data=None):
@validate_schema(certificate_edit_input_schema, certificate_output_schema)
def post(self, certificate_id, data=None):
"""
.. http:post:: /certificates/1/update/notify
.. http:post:: /certificates/1/update/switches

Update certificate notification
Update certificate boolean switches for notification or rotation

**Example request**:

.. sourcecode:: http

POST /certificates/1/update/notify HTTP/1.1
POST /certificates/1/update/switches HTTP/1.1
Host: example.com
Accept: application/json, text/javascript
Content-Type: application/json;charset=UTF-8

{
"notify": false
"notify": false,
"rotation": false
}

**Example response**:
Expand Down Expand Up @@ -980,6 +982,7 @@ def post(self, certificate_id, data=None):
"description": null,
"deleted": null,
"notify": false,
"rotation": false,
"notifications": [{
"id": 1
}]
Expand Down Expand Up @@ -1029,7 +1032,7 @@ def post(self, certificate_id, data=None):
403,
)

cert = service.update_notify(cert, data.get("notify"))
cert = service.update_switches(cert, notify_flag=data.get("notify"), rotation_flag=data.get("rotation"))
log_service.create(g.current_user, "update_cert", certificate=cert)
return cert

Expand Down Expand Up @@ -1516,7 +1519,7 @@ def put(self, certificate_id, data=None):
Certificates, "/certificates/<int:certificate_id>", endpoint="certificate"
)
api.add_resource(
Certificates, "/certificates/<int:certificate_id>/update/notify", endpoint="certificateUpdateNotify"
Certificates, "/certificates/<int:certificate_id>/update/switches", endpoint="certificateUpdateSwitches"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's likely that anyone's using this endpoint programmatically (i.e. not via the UI) right now -- and therefore I think this change is acceptable -- but I do at least want to call out that this is a breaking/backwards incompatible change if they are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jtschladen, that's a great point; we could bump the version in the changelog for it if desired! Although as you noted, I also suspect this isn't a highly used API endpoint given that PUT /certificates/<id> has been around for much longer, and offers more flexibility to programmatic callers looking to update certs.

I'd be happy to preserve /update/notify and add a new, separate /update/rotation endpoint, if we want to be cautious. I think I'd just need to add a new Resource class for it, split up the service funcs, and use a customPOST in the Angular service call.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @havron for contributing the change. Code changes look good to me. We do suspect that it's not a highly used endpoint, so consolidating new changes in existing API makes sense. Given backward incompatibility, bumping version and adding changelog calling out this change will be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me! I just added a changelog in 3e94c84, following the new release process. Let me know if I can improve the release notes 🙂 @charhate

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @havron, really appreciate your efforts in compiling detailed release notes.
Given all the change going in next release and specially API change that is not backward compatible, the minor version change seems more appropriate (so 0.10.0 instead of 0.9.1). I have taken liberty to commit this 7edee9f, hope you do not mind.
I'll merge the PR soon and do a release.

)
api.add_resource(CertificatesStats, "/certificates/stats", endpoint="certificateStats")
api.add_resource(
Expand Down
2 changes: 1 addition & 1 deletion lemur/static/app/angular/certificates/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ angular.module('lemur')
return certificate.customGET('key');
};

CertificateService.updateNotify = function (certificate) {
CertificateService.updateSwitches = function (certificate) {
return certificate.post();
};

Expand Down
21 changes: 20 additions & 1 deletion lemur/static/app/angular/certificates/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ angular.module('lemur')
};

$scope.updateNotify = function (certificate) {
CertificateService.updateNotify(certificate).then(
CertificateService.updateSwitches(certificate).then(
function () {
toaster.pop({
type: 'success',
Expand All @@ -133,6 +133,25 @@ angular.module('lemur')
certificate.notify = false;
});
};
$scope.updateRotation = function (certificate) {
CertificateService.updateSwitches(certificate).then(
function () {
toaster.pop({
type: 'success',
title: certificate.name,
body: 'Updated!'
});
},
function (response) {
toaster.pop({
type: 'error',
title: certificate.name,
body: 'Unable to update! ' + response.data.message,
timeout: 100000
});
certificate.rotation = false;
});
};
$scope.getCertificateStatus = function () {
var def = $q.defer();
def.resolve([{'title': 'True', 'id': true}, {'title': 'False', 'id': false}]);
Expand Down
6 changes: 6 additions & 0 deletions lemur/static/app/angular/certificates/view/view.tpl.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ <h2 class="featurette-heading">Certificates
<li>{{ certificate.name }}</li>
<li><span class="text-muted">{{ certificate.owner }}</span></li>
</ul>
</td>
<td data-title="'Autorotate'" filter="{ 'rotation': 'select' }" filter-data="getCertificateStatus()">
<form>
<switch ng-change="updateRotation(certificate)" id="rotation" name="rotation"
ng-model="certificate.rotation" class="green small"></switch>
</form>
</td>
<td data-title="'Notify'" filter="{ 'notify': 'select' }" filter-data="getCertificateStatus()">
<form>
Expand Down