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

Remove cs.type param #4744

Merged
merged 1 commit into from
May 15, 2024
Merged

Remove cs.type param #4744

merged 1 commit into from
May 15, 2024

Conversation

edewata
Copy link
Contributor

@edewata edewata commented May 14, 2024

The cs.type param has been removed from CS.cfg since subsystem type is not actually changeable and this param might introduce configuration issues.

The code that uses the subsystem type has been modified to call CMSEngine.getName() (for uppercase subsystem type) and getID() for (for lower case subsystem type) instead.

The PKISubsystem.create_conf() has been modified to no longer add the param if it's missing. The load() has also been updated to no longer read the param.

The upgrade script has been modified to remove the param from existing instances.

The cs.type param has been removed from CS.cfg since subsystem
type is not actually changeable and this param might introduce
configuration issues.

The code that uses the subsystem type has been modified to call
CMSEngine.getName() (for uppercase subsystem type) and getID()
for (for lower case subsystem type) instead.

The PKISubsystem.create_conf() has been modified to no longer
add the param if it's missing. The load() has also been updated
to no longer read the param.

The upgrade script has been modified to remove the param from
existing instances.
@edewata edewata requested a review from fmarco76 May 14, 2024 16:50
Copy link

Copy link
Member

@fmarco76 fmarco76 left a comment

Choose a reason for hiding this comment

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

LGTM
I am OK with the current change. I have some doubt with storing the name and id separately since they provide the same information and one is related to the other but it is out of this PR scope so feel free to merge.

@edewata
Copy link
Contributor Author

edewata commented May 15, 2024

@fmarco76 Thanks!

Yeah, in the past there was an idea to support multiple subsystems of the same type, so they will have different IDs (e.g. ca1, ca2) but the same type (e.g. CA). I don't think that is needed anymore so we can clean up this code later. If we need multiple CAs it should be handled by a single CA subsystem using multi-tenancy (i.e. ligtweight CA), so no new deployment is required.

@edewata edewata merged commit 0f7647f into dogtagpki:master May 15, 2024
142 of 147 checks passed
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