Skip to content

mgr/dasboard : Injest certificate mgmt API into services API#66465

Merged
Pegonzal merged 1 commit intoceph:mainfrom
rhcs-dashboard:inject-cert-ls-api
Jan 29, 2026
Merged

mgr/dasboard : Injest certificate mgmt API into services API#66465
Pegonzal merged 1 commit intoceph:mainfrom
rhcs-dashboard:inject-cert-ls-api

Conversation

@abhidesai6
Copy link
Copy Markdown

@abhidesai6 abhidesai6 commented Dec 1, 2025

fixes : https://tracker.ceph.com/issues/74039
Signed-off-by: Abhishek Desai abhishek.desai1@ibm.com

UTC :

Screencast.From.2025-12-11.13-45-42.mp4
Screencast.From.2025-12-11.13-48-55.mp4

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@abhidesai6 abhidesai6 marked this pull request as ready for review December 3, 2025 10:33
@abhidesai6 abhidesai6 requested a review from a team as a code owner December 3, 2025 10:33
@abhidesai6 abhidesai6 force-pushed the inject-cert-ls-api branch 4 times, most recently from d279321 to 61e284b Compare December 11, 2025 17:54
@abhidesai6 abhidesai6 requested a review from Pegonzal December 11, 2025 17:56
@abhidesai6 abhidesai6 assigned nizamial09 and unassigned nizamial09 Dec 11, 2025
Copy link
Copy Markdown
Contributor

@Pegonzal Pegonzal left a comment

Choose a reason for hiding this comment

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

Hey @abhidesai6 I just went over your PR a first time, I'll try to double check it (since it's a big PR) in case I missed anything important. But overall looks good to me, pretty good job.

I've left some code suggestions and some things I believe that should be changed, please let me know what you think.

Also, I see we are completly flattening the provided information, this is what I get when listing:

{
    "cert_name": "cephadm-signed_agent_cert",
    "scope": "HOST",
    "signed_by": "cephadm",
    "status": "valid",
    "days_to_expiration": 1803,
    "expiry_date": null,
    "issuer": null,
    "common_name": "192.168.100.100",
    "target": "ceph-node-00",
    "subject": {
      "commonName": "192.168.100.100"
    },
    "key_type": null,
    "key_size": null
  }

I see common_name and commonName under subject do we want to keep both?

@abhidesai6 abhidesai6 force-pushed the inject-cert-ls-api branch 2 times, most recently from 0f1aee7 to 111b69c Compare December 30, 2025 14:06
@abhidesai6
Copy link
Copy Markdown
Author

jenkins test make check

@abhidesai6
Copy link
Copy Markdown
Author

jenkins test make check arm64

1 similar comment
@abhidesai6
Copy link
Copy Markdown
Author

jenkins test make check arm64

Copy link
Copy Markdown
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.

@abhidesai6 : here is a first impression review. tbh this is not a full review since I couldn't go through the entire code in the services/certificate.py since it was quite big. For future references, if you think a code will grow bigger, it'd be better if you could split the PR into different PRs so that it'd make it easier for reviewers to give proper review and make it look less scarier :P

Comment on lines +210 to +217
def process_certificates_for_list(cert_ls_data: Dict[str, Any]
) -> List[Dict[str, Any]]:
"""
Process certificate list data and return formatted certificate entries.

:param cert_ls_data: Certificate list data from cert_ls
:return: List of certificate entry dictionaries
"""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just to understand the functionality of this function better, can you paste the before and after of the cert_ls result?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added Before and after of cert_ls_result to docString

:cert_ls_input:
            input: {
                'rgw_ssl_cert': {
                    'scope': 'service',
                    'certificates': {
                        'rgw.nashik': {
                            'subject': {
                                'organizationName': 'ceph',
                                'commonName': '192.168.100.100:8443'
                            },
                            'validity': {'remaining_days': -42}
                        }
                    }
                }
            }
            output: [{
                'cert_name': 'rgw_ssl_cert',
                'scope': 'SERVICE',
                'signed_by': 'user',
                'status': 'expired',
                'days_to_expiration': -42,
                'expiry_date': None,
                'issuer': None,
                'common_name': '192.168.100.100:8443',
                'target': 'rgw.nashik'
            }]
        """

@github-project-automation github-project-automation bot moved this from New to Review in progress in Ceph-Dashboard Jan 8, 2026
@abhidesai6 abhidesai6 force-pushed the inject-cert-ls-api branch 2 times, most recently from 434c15e to f7e1c21 Compare January 16, 2026 08:19
@abhidesai6 abhidesai6 force-pushed the inject-cert-ls-api branch 3 times, most recently from f50ba69 to 47b21a5 Compare January 16, 2026 12:35
@abhidesai6 abhidesai6 force-pushed the inject-cert-ls-api branch 4 times, most recently from 6054f90 to 51bcbcc Compare January 20, 2026 20:23
Copy link
Copy Markdown
Contributor

@Pegonzal Pegonzal left a comment

Choose a reason for hiding this comment

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

Hey @abhidesai6 pretty good job with this, I left a bunch of comments most are nitpicks and some are things that I believe could be improved, however the most relevant one would be around the methods used to select the services like I asked in the _select_service_certificate.

Would like to understand this better before giving the approval, I'll check this back tomorrow, and thanks again for the great work!

fixes : https://tracker.ceph.com/issues/74039
Signed-off-by: Abhishek Desai <abhishek.desai1@ibm.com>
Assisted-by: Cursor
Copy link
Copy Markdown
Contributor

@Pegonzal Pegonzal 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 for all the effort here @abhidesai6 , good job.

@Pegonzal Pegonzal dismissed nizamial09’s stale review January 29, 2026 09:11

Not enough bandwith to revisit, dm'ed to confirm is ok to proceed

@Pegonzal Pegonzal merged commit 1d76046 into ceph:main Jan 29, 2026
13 of 15 checks passed
@Pegonzal Pegonzal deleted the inject-cert-ls-api branch January 29, 2026 09:12
@github-project-automation github-project-automation bot moved this from Review in progress to Done in Ceph-Dashboard Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants