Skip to content

Fix certificate assignment for non-default services#381

Open
jmcte wants to merge 1 commit intoN4S4:masterfrom
OMT-Global:codex/issue-341-cert-service
Open

Fix certificate assignment for non-default services#381
jmcte wants to merge 1 commit intoN4S4:masterfrom
OMT-Global:codex/issue-341-cert-service

Conversation

@jmcte
Copy link
Copy Markdown
Contributor

@jmcte jmcte commented Apr 26, 2026

🗃️ Summary

Fix certificate assignment for DSM services that are discovered from the live certificate service list instead of the static service map.


🚀 Motivation & Problem Statement

Certificate.set_certificate_for_service() currently indexes a hardcoded service dictionary by service_name. That works for DSM Desktop Service, but it fails for non-default DSM services such as reverse proxy certificate assignments because those service entries are returned by DSM at runtime and are not present in the static map.


🔧 Implementation Details

  • Modified synology_api/core_certificate.py so set_certificate_for_service() captures the matching service object while scanning list_cert().
  • Kept the existing static payload behavior for known services such as DSM Desktop Service.
  • Added a fallback that uses the DSM-discovered service definition for non-default services.
  • Added a clear ValueError when the requested service is not present in the certificate service list.
  • Added focused unit tests in tests/test_core_certificate.py for default service behavior, discovered non-default service behavior, missing service errors, and already-assigned no-op behavior.

🏁 Checklist

  • I have read and followed the Contributing guidelines.
  • All new or modified code is covered by unit tests (tests/).
  • Tests pass locally (pytest).
  • I added or updated documentation where necessary.
  • I updated the changelog or added a new section if this is a major change.
  • I followed the style guidelines (black, flake8, etc.).
  • I ran pre-commit and addressed any linting issues.

pre-commit is not installed in the project venv used for local verification, so I could not run it locally. The GitHub pre-commit workflow is running on this PR.


📝 Related Issue

Closes #341.


🔨 Additional Notes

Live DSM verification was performed against reverse proxy certificate assignment:

  • Created a temporary reverse proxy service.
  • Changed its certificate assignment.
  • Verified the new assignment was reflected by DSM.
  • Restored the original certificate assignment.
  • Deleted the temporary reverse proxy service.

The marked safe integration tests currently fail for me even on clean upstream/master against my NAS, so I am treating those as existing environment/API-version drift rather than part of this change.


😎 Test & Build Status

python -m pytest tests/test_core_certificate.py tests/test_core_service_apps.py -q
# 64 passed in 0.09s

git diff --check
# passed

Attempted but unavailable locally:

pre-commit run --all-files
# no such file or directory

python -m pre_commit run --all-files
# No module named pre_commit

👀 Screenshots / Media

Not applicable.


Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Certificate.set_certificate_for_service limiting possible services, not allowing reverse proxy services

1 participant