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

mgr/dashboard: remove usage of 'rgw_frontend_ssl_key' #42305

Merged
merged 1 commit into from Jul 13, 2021

Conversation

avanthakkar
Copy link
Contributor

@avanthakkar avanthakkar commented Jul 13, 2021

Fixes: https://tracker.ceph.com/issues/51643
Signed-off-by: Avan Thakkar athakkar@redhat.com

Removing the usage of rgw_frontend_ssl_key from the rgw service form.

Before:
Screenshot from 2021-07-13 18-16-13

After:
Screenshot from 2021-07-13 18-08-17

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@avanthakkar avanthakkar requested review from epuertat and a team July 13, 2021 12:43
@avanthakkar avanthakkar added this to In progress in Dashboard via automation Jul 13, 2021
@avanthakkar avanthakkar requested review from pereman2 and aaryanporwal and removed request for a team July 13, 2021 12:43
Dashboard automation moved this from In progress to Reviewer approved Jul 13, 2021
Copy link
Member

@nizamial09 nizamial09 left a comment

Choose a reason for hiding this comment

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

LGTM! apart from one small question. Thanks @avanthakkar (probably a unit test to make sure its not showing for type rgws)

@avanthakkar
Copy link
Contributor Author

LGTM! apart from one small question. Thanks @avanthakkar (probably a unit test to make sure its not showing for type rgws)

But we don't pass the ssl_key itself so Idk what to test here. It's just the visual difference you will see as I have attached the screenshot in PR description.

@avanthakkar avanthakkar requested a review from a team as a code owner July 13, 2021 12:57
@nizamial09
Copy link
Member

nizamial09 commented Jul 13, 2021

what to test here

@avanthakkar sorry i wasnt being clear, I was actually asking to test whether the field is present or not. Something like this in the rgw test area of the service component.


      it('should not show private key field', () => {
        formHelper.setValue('ssl', true);
        fixture.detectChanges();
        const ssl_key = fixture.debugElement.query(By.css('#ssl_key'));
        expect(ssl_key).toBeNull();
      })

Fixes: https://tracker.ceph.com/issues/51643
Signed-off-by: Avan Thakkar <athakkar@redhat.com>

Removing the usage of rgw_frontend_ssl_key from the rgw service form.
@avanthakkar
Copy link
Contributor Author

what to test here

@avanthakkar sorry i wasnt being clear, I was actually asking to test whether the field is present or not. Something like this in the rgw test area of the service component.


      it('should not show private key field', () => {
        formHelper.setValue('ssl', true);
        fixture.detectChanges();
        const ssl_key = fixture.debugElement.query(By.css('#ssl_key'));
        expect(ssl_key).toBeNull();
      })

Yes you're right. Thanks @nizamial09 , updated the changes.

Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

LGTM Thanks @avanthakkar !

Comment on lines 510 to +511
<!-- ssl_key -->
<div *ngIf="serviceForm.controls.ssl.value"
<div *ngIf="serviceForm.controls.ssl.value && serviceForm.controls.service_type.value !== 'rgw'"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we somehow indicate the user that they're expected to submit a concatenated certificate + key (.PEM-like format)?

@epuertat epuertat merged commit 57907eb into ceph:master Jul 13, 2021
Dashboard automation moved this from Reviewer approved to Done Jul 13, 2021
@epuertat epuertat deleted the remove-rgw-frontend-ssl-key branch July 13, 2021 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Dashboard
  
Done
3 participants