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

Restart the instance when new subsystems are deployed #4571

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

fmarco76
Copy link
Member

@fmarco76 fmarco76 commented Sep 20, 2023

If a new subsystem is deployed in a existing instance this not restarted during the installation but the new web-app is just enabled.

When the subsystem are configured to work with an HSM this could generate problems because certificates added during the installation with external tools are recognised. The instance restart will clean the internal cache and reference to the HSM and all certificates are identified.

Fix the issue #4335

@fmarco76 fmarco76 force-pushed the HsmKRADeployShareInstance branch from 46f5b01 to 6520837 Compare September 20, 2023 15:32
@edewata
Copy link
Contributor

edewata commented Sep 21, 2023

So the original code works like this:

if this is the first subsystem
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is the first subsystem:
    if using legacy ID generator:
        stop the server
        remove temp SSL server cert
    use permanent SSL server cert
else:
    enable subsystem (without restarting the server)

if this is the first subsystem:
    start the server

IIUC the proposed code works like this:

if this is the first subsystem:
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is not the first subsystem or using legacy ID generator:
    stop the server

if this is the first subsystem:
    if using legacy ID generator
        remove temp SSL server cert
    use permanent SSL server cert

start the server

I think the above code works, but I'm wondering if we can avoid restarting the server if it doesn't use HSM (because that would be faster), so something like this:

if this is the first subsystem:
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is the first subsystem:
    if using legacy ID generator:
        stop the server
        remove temp SSL server cert
    use permanent SSL server cert
else:
    if using HSM:
        restart the server (which will automatically enable the subsystem)
    else:
        enable subsystem (without restarting the server)

if this is the first subsystem:
    start the server

It doesn't have to be exactly like this, but that's the idea. What do you think?

@fmarco76 fmarco76 force-pushed the HsmKRADeployShareInstance branch 2 times, most recently from 41708de to 5708f3e Compare September 21, 2023 10:40
If a new subsystem is deployed in a existing instance this not restarted
during the installation but the new web-app is just enabled.

When the subsystem are configured to work with an HSM this could
generate problems because certificates added during the installation
with external tools are recognised. The instance restart will clean the
internal cache and reference to the HSM and all certificates are
identified.

Fix the issue dogtagpki#4335
@fmarco76 fmarco76 force-pushed the HsmKRADeployShareInstance branch from 5708f3e to a7e7613 Compare September 21, 2023 11:40
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@fmarco76
Copy link
Member Author

So the original code works like this:

if this is the first subsystem
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is the first subsystem:
    if using legacy ID generator:
        stop the server
        remove temp SSL server cert
    use permanent SSL server cert
else:
    enable subsystem (without restarting the server)

if this is the first subsystem:
    start the server

IIUC the proposed code works like this:

if this is the first subsystem:
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is not the first subsystem or using legacy ID generator:
    stop the server

if this is the first subsystem:
    if using legacy ID generator
        remove temp SSL server cert
    use permanent SSL server cert

start the server

I think the above code works, but I'm wondering if we can avoid restarting the server if it doesn't use HSM (because that would be faster), so something like this:

if this is the first subsystem:
    enable subsystem (which might be redundant)
    if using legacy ID generator:
        create temp SSL server cert
        start the server

if this is the first subsystem:
    if using legacy ID generator:
        stop the server
        remove temp SSL server cert
    use permanent SSL server cert
else:
    if using HSM:
        restart the server (which will automatically enable the subsystem)
    else:
        enable subsystem (without restarting the server)

if this is the first subsystem:
    start the server

It doesn't have to be exactly like this, but that's the idea. What do you think?

I have updated the code to be in line with this idea.

Copy link
Contributor

@edewata edewata left a comment

Choose a reason for hiding this comment

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

Thanks for the update! LGTM.

@fmarco76
Copy link
Member Author

@edewata Thanks!

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.

2 participants