Skip to content

CLOUDSTACK-9815 move CertService to more generic location #2071

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.

Sign up for GitHub

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

Merged
karuturi merged 3 commits into from
May 2, 2017
Merged

CLOUDSTACK-9815 move CertService to more generic location #2071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bb7ffe
CLOUDSTACK-9815 move CertService to more generic location
DaanHoogland Apr 27, 2017
20c67ac
CLOUDSTACK-9815 rename ssl to tls
DaanHoogland Apr 27, 2017
ae4e571
CLOUDSTACK-9815 move package further to apache location
DaanHoogland May 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteSslCertCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.lb.CertService;
import org.apache.cloudstack.network.tls.CertService;
import com.cloud.utils.exception.CloudRuntimeException;

@APICommand(name = "deleteSslCert", description = "Delete a certificate to CloudStack", responseObject = SuccessResponse.class,
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/apache/cloudstack/api/command/user/loadbalancer/ListSslCertsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.apache.cloudstack.api.response.SslCertResponse;
import org.apache.cloudstack.context.CallContext;

import com.cloud.network.lb.CertService;
import org.apache.cloudstack.network.tls.CertService;
import com.cloud.utils.exception.CloudRuntimeException;

@APICommand(name = "listSslCerts", description = "Lists SSL certificates", responseObject = SslCertResponse.class,
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/apache/cloudstack/api/command/user/loadbalancer/UploadSslCertCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.lb.CertService;
import org.apache.cloudstack.network.tls.CertService;

@APICommand(name = "uploadSslCert", description = "Upload a certificate to CloudStack", responseObject = SslCertResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/apache/cloudstack/api/response/SslCertResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.apache.cloudstack.api.BaseResponse;
import org.apache.cloudstack.api.EntityReference;

import com.cloud.network.lb.SslCert;
import org.apache.cloudstack.network.tls.SslCert;
import com.cloud.serializer.Param;

//import org.apache.cloudstack.api.EntityReference;
Expand Down
5 changes: 4 additions & 1 deletion ...src/com/cloud/network/lb/CertService.java → ...e/cloudstack/network/tls/CertService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package com.cloud.network.lb;
package org.apache.cloudstack.network.tls;

import java.security.cert.Certificate;
import java.util.List;

import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
Expand All @@ -30,4 +31,6 @@ public interface CertService {
public void deleteSslCert(DeleteSslCertCmd deleteSslCertCmd);

public List<SslCertResponse> listSslCerts(ListSslCertsCmd listSslCertCmd);

public Certificate parseCertificate(final String cert);
}
2 changes: 1 addition & 1 deletion api/src/com/cloud/network/lb/SslCert.java → ...pache/cloudstack/network/tls/SslCert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package com.cloud.network.lb;
package org.apache.cloudstack.network.tls;

import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.api.Identity;
Expand Down
2 changes: 1 addition & 1 deletion engine/schema/src/com/cloud/network/dao/SslCertVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import javax.persistence.Id;
import javax.persistence.Table;

import com.cloud.network.lb.SslCert;
import org.apache.cloudstack.network.tls.SslCert;
import com.cloud.utils.db.Encrypt;

@Entity
Expand Down
2 changes: 1 addition & 1 deletion server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,6 @@
class="org.apache.cloudstack.region.gslb.GlobalLoadBalancingRulesServiceImpl" >
<property name="gslbServiceProviders" value="#{gslbServiceProvidersRegistry.registered}" />
</bean>
<bean id="certServiceImpl" class="org.apache.cloudstack.network.lb.CertServiceImpl" />
<bean id="certServiceImpl" class="org.apache.cloudstack.network.ssl.CertServiceImpl" />
Copy link
Member

@rafaelweingartner rafaelweingartner May 1, 2017

Choose a reason for hiding this comment

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

shouldn't this package be "org.apache.cloudstack.network.tls"?

Copy link
Contributor Author

@DaanHoogland DaanHoogland May 1, 2017

Choose a reason for hiding this comment

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

could be but that would in this state be incorrect (see other discuss location in this review thread.

Copy link
Member

@rohityadavcloud rohityadavcloud May 2, 2017

Choose a reason for hiding this comment

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

@karuturi the interfaces are in 'tls' sub-package, but the impl are still in 'ssl'?

Copy link
Contributor Author

@DaanHoogland DaanHoogland May 2, 2017

Choose a reason for hiding this comment

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

@rhtyd we will address that when we make this into a CA-service. I'll create a ticket. It is really very minor.

Copy link
Contributor Author

@DaanHoogland DaanHoogland May 2, 2017

Choose a reason for hiding this comment

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

issue CLOUDSTACK-9898 created

<bean id="imageStoreUploadMonitorImpl" class="com.cloud.storage.ImageStoreUploadMonitorImpl" />
</beans>
4 changes: 2 additions & 2 deletions ...loudstack/network/lb/CertServiceImpl.java → ...oudstack/network/ssl/CertServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package org.apache.cloudstack.network.lb;
package org.apache.cloudstack.network.ssl;
Copy link
Member

@rafaelweingartner rafaelweingartner May 1, 2017

Choose a reason for hiding this comment

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

I got a little confuse here.
Did you create "org.apache.cloudstack.network.tls" or "org.apache.cloudstack.network.ssl"?

Copy link
Contributor Author

@DaanHoogland DaanHoogland May 1, 2017

Choose a reason for hiding this comment

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

ae4e571#diff-ebb55a3f7379d7132877a94057342689R39 shows "org.apache.cloudstack.network.tls". I will have a quick check to see what happened locally?

Copy link
Contributor Author

@DaanHoogland DaanHoogland May 1, 2017

Choose a reason for hiding this comment

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

So here is what happened, @rafaelweingartner :
The service definition was in cloud.com... the service implementation was (already) in org.apache.cloudstack...
then I moved the definition around because of your and @resmo 's comments.
Now I could of course take a few seconds to move this code but I won't right now. I am in the process of building on it and will get to that in a next iteration.

Copy link
Member

@rafaelweingartner rafaelweingartner May 1, 2017

Choose a reason for hiding this comment

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

Ah, now I get it. So, someone else in the past moved the implementation, but not the interfaces.
That is why I was confused, thanks for the explanation and the good work 👍


import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
Expand All @@ -26,7 +26,7 @@
import com.cloud.network.dao.LoadBalancerVO;
import com.cloud.network.dao.SslCertDao;
import com.cloud.network.dao.SslCertVO;
import com.cloud.network.lb.CertService;
import org.apache.cloudstack.network.tls.CertService;
import com.cloud.network.rules.LoadBalancer;
import com.cloud.projects.Project;
import com.cloud.projects.ProjectService;
Expand Down
151 changes: 74 additions & 77 deletions ...loudstack/network/lb/CertServiceTest.java → ...oudstack/network/ssl/CertServiceTest.java
View file
Open in desktop

Large diffs are not rendered by default.