From a1faf09a2c1d860d5b3c82c0680ac5e69563e33f Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Wed, 17 Jul 2019 11:43:40 -0300 Subject: [PATCH 01/10] ui: Fix SystemVMs public range dedication (#3495) Fix small UI issue when dedicating public IR range for system VMs: Unable to execute API command createvlaniprange due to invalid value. Invalid parameter domainid value=undefined due to incorrect long value format, or entity does not exist or due to incorrect parameter annotation for the field in api cmd class. Fixes: #3485 --- ui/scripts/system.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/scripts/system.js b/ui/scripts/system.js index c3f7a2bc4676..f6ef03ca9d3e 100755 --- a/ui/scripts/system.js +++ b/ui/scripts/system.js @@ -496,11 +496,13 @@ if (args.data.account) { if (args.data.account.account) array1.push("&account=" + args.data.account.account); + if (args.data.account.domainid) { + array1.push("&domainid=" + args.data.account.domainid); + } if (args.data.account.systemvms) { systvmsval = args.data.account.systemvms == "on" ? "true" : "false" array1.push("&forsystemvms=" + systvmsval); } - array1.push("&domainid=" + args.data.account.domainid); } array1.push("&forVirtualNetwork=true"); From a1a9fb8977532f9d43c78b785d0bf3e9c53fc66e Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Thu, 18 Jul 2019 02:39:00 -0300 Subject: [PATCH 02/10] KVM: Enhancements for direct download feature (#3374) * Add revoke certificates API * Add background task to sync certificates * Fix marvin test and revoke certificate * Fix certificate sent to hypervisor was missing headers * Fix background task for uploading certificates to hosts --- ...eTemplateDirectDownloadCertificateCmd.java | 98 ++++++++ ...dTemplateDirectDownloadCertificateCmd.java | 12 +- .../download/DirectDownloadCertificate.java | 29 +++ .../download/DirectDownloadManager.java | 16 +- ...evokeDirectDownloadCertificateCommand.java | 39 +++ .../main/java/com/cloud/host/dao/HostDao.java | 2 + .../java/com/cloud/host/dao/HostDaoImpl.java | 10 + .../DirectDownloadCertificateDao.java | 27 ++ .../DirectDownloadCertificateDaoImpl.java | 53 ++++ .../DirectDownloadCertificateHostMapDao.java | 26 ++ ...rectDownloadCertificateHostMapDaoImpl.java | 48 ++++ .../DirectDownloadCertificateHostMapVO.java | 84 +++++++ .../download/DirectDownloadCertificateVO.java | 119 +++++++++ ...spring-engine-schema-core-daos-context.xml | 2 + .../META-INF/db/schema-41200to41300.sql | 27 ++ .../download/DirectDownloadService.java | 7 +- ...evokeDirectDownloadCertificateWrapper.java | 106 ++++++++ .../cloud/server/ManagementServerImpl.java | 2 + .../download/DirectDownloadManagerImpl.java | 232 ++++++++++++++++-- .../integration/smoke/test_direct_download.py | 15 +- 20 files changed, 933 insertions(+), 21 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/RevokeTemplateDirectDownloadCertificateCmd.java create mode 100644 api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificate.java create mode 100644 core/src/main/java/org/apache/cloudstack/agent/directdownload/RevokeDirectDownloadCertificateCommand.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDao.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDaoImpl.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDao.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDaoImpl.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapVO.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateVO.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/RevokeTemplateDirectDownloadCertificateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/RevokeTemplateDirectDownloadCertificateCmd.java new file mode 100644 index 000000000000..ef9fa8b1fa28 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/RevokeTemplateDirectDownloadCertificateCmd.java @@ -0,0 +1,98 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package org.apache.cloudstack.api.command.admin.direct.download; + +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.NetworkRuleConflictException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.HostResponse; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.direct.download.DirectDownloadManager; +import org.apache.log4j.Logger; + +import javax.inject.Inject; + +@APICommand(name = RevokeTemplateDirectDownloadCertificateCmd.APINAME, + description = "Revoke a certificate alias from a KVM host", + responseObject = SuccessResponse.class, + requestHasSensitiveInfo = true, + responseHasSensitiveInfo = true, + since = "4.13", + authorized = {RoleType.Admin}) +public class RevokeTemplateDirectDownloadCertificateCmd extends BaseCmd { + + @Inject + DirectDownloadManager directDownloadManager; + + private static final Logger LOG = Logger.getLogger(RevokeTemplateDirectDownloadCertificateCmd.class); + public static final String APINAME = "revokeTemplateDirectDownloadCertificate"; + + @Parameter(name = ApiConstants.NAME, type = BaseCmd.CommandType.STRING, required = true, + description = "alias of the SSL certificate") + private String certificateAlias; + + @Parameter(name = ApiConstants.HYPERVISOR, type = BaseCmd.CommandType.STRING, required = true, + description = "hypervisor type") + private String hypervisor; + + @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, + description = "zone to revoke certificate", required = true) + private Long zoneId; + + @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class, + description = "(optional) the host ID to revoke certificate") + private Long hostId; + + @Override + public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { + if (!hypervisor.equalsIgnoreCase("kvm")) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Currently supporting KVM hosts only"); + } + SuccessResponse response = new SuccessResponse(getCommandName()); + try { + LOG.debug("Revoking certificate " + certificateAlias + " from " + hypervisor + " hosts"); + boolean result = directDownloadManager.revokeCertificateAlias(certificateAlias, hypervisor, zoneId, hostId); + response.setSuccess(result); + setResponseObject(response); + } catch (Exception e) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage()); + } + } + + @Override + public String getCommandName() { + return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX; + } + + @Override + public long getEntityOwnerId() { + return CallContext.current().getCallingAccount().getId(); + } +} \ No newline at end of file diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java index c93fca2d300d..223f20b5bb0d 100755 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java @@ -23,7 +23,9 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.direct.download.DirectDownloadManager; import org.apache.log4j.Logger; @@ -56,6 +58,14 @@ public class UploadTemplateDirectDownloadCertificateCmd extends BaseCmd { @Parameter(name = ApiConstants.HYPERVISOR, type = BaseCmd.CommandType.STRING, required = true, description = "Hypervisor type") private String hypervisor; + @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, + description = "Zone to upload certificate", required = true) + private Long zoneId; + + @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class, + description = "(optional) the host ID to revoke certificate") + private Long hostId; + @Override public void execute() { if (!hypervisor.equalsIgnoreCase("kvm")) { @@ -64,7 +74,7 @@ public void execute() { try { LOG.debug("Uploading certificate " + name + " to agents for Direct Download"); - boolean result = directDownloadManager.uploadCertificateToHosts(certificate, name, hypervisor); + boolean result = directDownloadManager.uploadCertificateToHosts(certificate, name, hypervisor, zoneId, hostId); SuccessResponse response = new SuccessResponse(getCommandName()); response.setSuccess(result); setResponseObject(response); diff --git a/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificate.java b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificate.java new file mode 100644 index 000000000000..6227c26ceab4 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificate.java @@ -0,0 +1,29 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.hypervisor.Hypervisor; +import org.apache.cloudstack.api.Identity; +import org.apache.cloudstack.api.InternalIdentity; + +public interface DirectDownloadCertificate extends InternalIdentity, Identity { + + String getCertificate(); + String getAlias(); + Hypervisor.HypervisorType getHypervisorType(); + +} \ No newline at end of file diff --git a/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java index b3f0841a6e89..d627ffa69d18 100644 --- a/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java +++ b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java @@ -19,7 +19,21 @@ import com.cloud.utils.component.PluggableService; import org.apache.cloudstack.framework.agent.direct.download.DirectDownloadService; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; -public interface DirectDownloadManager extends DirectDownloadService, PluggableService { +public interface DirectDownloadManager extends DirectDownloadService, PluggableService, Configurable { + ConfigKey DirectDownloadCertificateUploadInterval = new ConfigKey<>("Advanced", Long.class, + "direct.download.certificate.background.task.interval", + "0", + "This interval (in hours) controls a background task to sync hosts within enabled zones " + + "missing uploaded certificates for direct download." + + "Only certificates which have not been revoked from hosts are uploaded", + false); + + /** + * Revoke direct download certificate with alias 'alias' from hosts of hypervisor type 'hypervisor' + */ + boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId, Long hostId); } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/RevokeDirectDownloadCertificateCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/RevokeDirectDownloadCertificateCommand.java new file mode 100644 index 000000000000..b0eb98647dc3 --- /dev/null +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/RevokeDirectDownloadCertificateCommand.java @@ -0,0 +1,39 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package org.apache.cloudstack.agent.directdownload; + +import com.cloud.agent.api.Command; + +public class RevokeDirectDownloadCertificateCommand extends Command { + + private String certificateAlias; + + public RevokeDirectDownloadCertificateCommand(final String alias) { + this.certificateAlias = alias; + } + + public String getCertificateAlias() { + return certificateAlias; + } + + @Override + public boolean executeInSequence() { + return false; + } +} \ No newline at end of file diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java index 1fca86ca319f..dd45c0987adf 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java @@ -107,4 +107,6 @@ public interface HostDao extends GenericDao, StateDao listAllHostsUpByZoneAndHypervisor(long zoneId, HypervisorType hypervisorType); } diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java index 8c8c082ed8f6..71f0aef39d67 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Objects; import java.util.TimeZone; +import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.inject.Inject; @@ -1190,6 +1191,15 @@ public HostVO findHostInZoneToExecuteCommand(long zoneId, HypervisorType hypervi } } + @Override + public List listAllHostsUpByZoneAndHypervisor(long zoneId, HypervisorType hypervisorType) { + return listByDataCenterIdAndHypervisorType(zoneId, hypervisorType) + .stream() + .filter(x -> x.getStatus().equals(Status.Up) && + x.getType() == Host.Type.Routing) + .collect(Collectors.toList()); + } + private ResultSet executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(HypervisorType hypervisorType, long zoneId, TransactionLegacy tx, String sql) throws SQLException { PreparedStatement pstmt = tx.prepareAutoCloseStatement(sql); pstmt.setString(1, Objects.toString(hypervisorType)); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDao.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDao.java new file mode 100644 index 000000000000..69f79e0448e0 --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDao.java @@ -0,0 +1,27 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.hypervisor.Hypervisor; +import com.cloud.utils.db.GenericDao; + +import java.util.List; + +public interface DirectDownloadCertificateDao extends GenericDao { + DirectDownloadCertificateVO findByAlias(String alias, Hypervisor.HypervisorType hypervisorType, long zoneId); + List listByZone(long zoneId); +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDaoImpl.java new file mode 100644 index 000000000000..a936cbb4490b --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDaoImpl.java @@ -0,0 +1,53 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.hypervisor.Hypervisor; +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; + +import java.util.List; + +public class DirectDownloadCertificateDaoImpl extends GenericDaoBase implements DirectDownloadCertificateDao { + + private final SearchBuilder certificateSearchBuilder; + + public DirectDownloadCertificateDaoImpl() { + certificateSearchBuilder = createSearchBuilder(); + certificateSearchBuilder.and("alias", certificateSearchBuilder.entity().getAlias(), SearchCriteria.Op.EQ); + certificateSearchBuilder.and("hypervisor_type", certificateSearchBuilder.entity().getHypervisorType(), SearchCriteria.Op.EQ); + certificateSearchBuilder.and("zone_id", certificateSearchBuilder.entity().getZoneId(), SearchCriteria.Op.EQ); + certificateSearchBuilder.done(); + } + + @Override + public DirectDownloadCertificateVO findByAlias(String alias, Hypervisor.HypervisorType hypervisorType, long zoneId) { + SearchCriteria sc = certificateSearchBuilder.create(); + sc.setParameters("alias", alias); + sc.setParameters("hypervisor_type", hypervisorType); + sc.setParameters("zone_id", zoneId); + return findOneBy(sc); + } + + @Override + public List listByZone(long zoneId) { + SearchCriteria sc = certificateSearchBuilder.create(); + sc.setParameters("zone_id", zoneId); + return listBy(sc); + } +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDao.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDao.java new file mode 100644 index 000000000000..e119b1d491ed --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDao.java @@ -0,0 +1,26 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.utils.db.GenericDao; + +import java.util.List; + +public interface DirectDownloadCertificateHostMapDao extends GenericDao { + DirectDownloadCertificateHostMapVO findByCertificateAndHost(long certificateId, long hostId); + List listByCertificateId(long certificateId); +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDaoImpl.java new file mode 100644 index 000000000000..7a0b732bbfd4 --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDaoImpl.java @@ -0,0 +1,48 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; + +import java.util.List; + +public class DirectDownloadCertificateHostMapDaoImpl extends GenericDaoBase implements DirectDownloadCertificateHostMapDao { + private final SearchBuilder mapSearchBuilder; + + public DirectDownloadCertificateHostMapDaoImpl() { + mapSearchBuilder = createSearchBuilder(); + mapSearchBuilder.and("certificate_id", mapSearchBuilder.entity().getCertificateId(), SearchCriteria.Op.EQ); + mapSearchBuilder.and("host_id", mapSearchBuilder.entity().getHostId(), SearchCriteria.Op.EQ); + mapSearchBuilder.done(); + } + @Override + public DirectDownloadCertificateHostMapVO findByCertificateAndHost(long certificateId, long hostId) { + SearchCriteria sc = mapSearchBuilder.create(); + sc.setParameters("certificate_id", certificateId); + sc.setParameters("host_id", hostId); + return findOneBy(sc); + } + + @Override + public List listByCertificateId(long certificateId) { + SearchCriteria sc = mapSearchBuilder.create(); + sc.setParameters("certificate_id", certificateId); + return listBy(sc); + } +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapVO.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapVO.java new file mode 100644 index 000000000000..db5faf669ffa --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapVO.java @@ -0,0 +1,84 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +@Entity +@Table(name = "direct_download_certificate_host_map") +public class DirectDownloadCertificateHostMapVO { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id; + + @Column(name = "host_id") + private Long hostId; + + @Column(name = "certificate_id") + private Long certificateId; + + @Column(name = "revoked") + private Boolean revoked; + + public DirectDownloadCertificateHostMapVO() { + } + + public DirectDownloadCertificateHostMapVO(Long certificateId, Long hostId) { + this.certificateId = certificateId; + this.hostId = hostId; + this.revoked = false; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public Long getHostId() { + return hostId; + } + + public void setHostId(Long hostId) { + this.hostId = hostId; + } + + public Long getCertificateId() { + return certificateId; + } + + public void setCertificateId(Long certificateId) { + this.certificateId = certificateId; + } + + public Boolean isRevoked() { + return revoked; + } + + public void setRevoked(Boolean revoked) { + this.revoked = revoked; + } +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateVO.java b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateVO.java new file mode 100644 index 000000000000..0b147d7a2273 --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateVO.java @@ -0,0 +1,119 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.direct.download; + +import com.cloud.hypervisor.Hypervisor; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import java.util.UUID; + +@Entity +@Table(name = "direct_download_certificate") +public class DirectDownloadCertificateVO implements DirectDownloadCertificate { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id; + + @Column(name = "uuid") + private String uuid; + + @Column(name = "alias") + private String alias; + + @Column(name = "certificate", length = 65535) + private String certificate; + + @Column(name = "hypervisor_type") + private Hypervisor.HypervisorType hypervisorType; + + @Column(name = "zone_id") + private Long zoneId; + + public DirectDownloadCertificateVO() { + this.uuid = UUID.randomUUID().toString(); + } + + public void setId(Long id) { + this.id = id; + } + + public void setUuid(String uuid) { + this.uuid = uuid; + } + + public void setAlias(String alias) { + this.alias = alias; + } + + public void setCertificate(String certificate) { + this.certificate = certificate; + } + + public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) { + this.hypervisorType = hypervisorType; + } + + public DirectDownloadCertificateVO(String alias, String certificate, + Hypervisor.HypervisorType hypervisorType, Long zoneId) { + this(); + this.alias = alias; + this.certificate = certificate; + this.hypervisorType = hypervisorType; + this.zoneId = zoneId; + } + + @Override + public String getCertificate() { + return certificate; + } + + @Override + public String getAlias() { + return alias; + } + + @Override + public Hypervisor.HypervisorType getHypervisorType() { + return hypervisorType; + } + + @Override + public long getId() { + return id; + } + + @Override + public String getUuid() { + return uuid; + } + + public Long getZoneId() { + return zoneId; + } + + public void setZoneId(Long zoneId) { + this.zoneId = zoneId; + } + +} \ No newline at end of file diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index fc2a752307d9..1cea7aa4b119 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -285,4 +285,6 @@ + + diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql b/engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql index 8b60592f2751..904e76e8df06 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql @@ -359,3 +359,30 @@ CREATE VIEW `cloud`.`project_view` AS `cloud`.`account` ON account.id = project_account.account_id left join `cloud`.`project_account` pacct ON projects.id = pacct.project_id; + +-- KVM: Add background task to upload certificates for direct download +CREATE TABLE `cloud`.`direct_download_certificate` ( + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, + `uuid` varchar(40) NOT NULL, + `alias` varchar(255) NOT NULL, + `certificate` text NOT NULL, + `hypervisor_type` varchar(45) NOT NULL, + `zone_id` bigint(20) unsigned NOT NULL, + PRIMARY KEY (`id`), + KEY `i_direct_download_certificate_alias` (`alias`), + KEY `fk_direct_download_certificate__zone_id` (`zone_id`), + CONSTRAINT `fk_direct_download_certificate__zone_id` FOREIGN KEY (`zone_id`) REFERENCES `data_center` (`id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +CREATE TABLE `cloud`.`direct_download_certificate_host_map` ( + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, + `certificate_id` bigint(20) unsigned NOT NULL, + `host_id` bigint(20) unsigned NOT NULL, + `revoked` int(1) NOT NULL DEFAULT 0, + PRIMARY KEY (`id`), + KEY `fk_direct_download_certificate_host_map__host_id` (`host_id`), + KEY `fk_direct_download_certificate_host_map__certificate_id` (`certificate_id`), + CONSTRAINT `fk_direct_download_certificate_host_map__host_id` FOREIGN KEY (`host_id`) REFERENCES `host` (`id`) ON DELETE CASCADE, + CONSTRAINT `fk_direct_download_certificate_host_map__certificate_id` FOREIGN KEY (`certificate_id`) REFERENCES `direct_download_certificate` (`id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + diff --git a/framework/direct-download/src/main/java/org/apache/cloudstack/framework/agent/direct/download/DirectDownloadService.java b/framework/direct-download/src/main/java/org/apache/cloudstack/framework/agent/direct/download/DirectDownloadService.java index f3153e3470e9..ed7bbd76a351 100644 --- a/framework/direct-download/src/main/java/org/apache/cloudstack/framework/agent/direct/download/DirectDownloadService.java +++ b/framework/direct-download/src/main/java/org/apache/cloudstack/framework/agent/direct/download/DirectDownloadService.java @@ -27,5 +27,10 @@ public interface DirectDownloadService { /** * Upload client certificate to each running host */ - boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor); + boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor, Long zoneId, Long hostId); + + /** + * Upload a stored certificate on database with id 'certificateId' to host with id 'hostId' + */ + boolean uploadCertificate(long certificateId, long hostId); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java new file mode 100644 index 000000000000..e942dcbad00e --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java @@ -0,0 +1,106 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.PropertiesUtil; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.cloudstack.agent.directdownload.RevokeDirectDownloadCertificateCommand; +import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.log4j.Logger; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; + +import static org.apache.commons.lang.StringUtils.isBlank; + +@ResourceWrapper(handles = RevokeDirectDownloadCertificateCommand.class) +public class LibvirtRevokeDirectDownloadCertificateWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(LibvirtRevokeDirectDownloadCertificateWrapper.class); + + /** + * Retrieve agent.properties file + */ + private File getAgentPropertiesFile() throws FileNotFoundException { + final File agentFile = PropertiesUtil.findConfigFile("agent.properties"); + if (agentFile == null) { + throw new FileNotFoundException("Failed to find agent.properties file"); + } + return agentFile; + } + + /** + * Get the property 'keystore.passphrase' value from agent.properties file + */ + private String getKeystorePassword(File agentFile) { + String pass = null; + if (agentFile != null) { + try { + pass = PropertiesUtil.loadFromFile(agentFile).getProperty(KeyStoreUtils.KS_PASSPHRASE_PROPERTY); + } catch (IOException e) { + s_logger.error("Could not get 'keystore.passphrase' property value due to: " + e.getMessage()); + } + } + return pass; + } + + /** + * Get keystore path + */ + private String getKeyStoreFilePath(File agentFile) { + return agentFile.getParent() + "/" + KeyStoreUtils.KS_FILENAME; + } + + @Override + public Answer execute(RevokeDirectDownloadCertificateCommand command, LibvirtComputingResource serverResource) { + String certificateAlias = command.getCertificateAlias(); + try { + File agentFile = getAgentPropertiesFile(); + String privatePassword = getKeystorePassword(agentFile); + if (isBlank(privatePassword)) { + return new Answer(command, false, "No password found for keystore: " + KeyStoreUtils.KS_FILENAME); + } + + final String keyStoreFile = getKeyStoreFilePath(agentFile); + + String checkCmd = String.format("keytool -list -alias %s -keystore %s -storepass %s", + certificateAlias, keyStoreFile, privatePassword); + int existsCmdResult = Script.runSimpleBashScriptForExitValue(checkCmd); + if (existsCmdResult == 1) { + s_logger.error("Certificate alias " + certificateAlias + " does not exist, no need to revoke it"); + } else { + String revokeCmd = String.format("keytool -delete -alias %s -keystore %s -storepass %s", + certificateAlias, keyStoreFile, privatePassword); + s_logger.debug("Revoking certificate alias " + certificateAlias + " from keystore " + keyStoreFile); + Script.runSimpleBashScriptForExitValue(revokeCmd); + } + } catch (FileNotFoundException | CloudRuntimeException e) { + s_logger.error("Error while setting up certificate " + certificateAlias, e); + return new Answer(command, false, e.getMessage()); + } + return new Answer(command); + } +} diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 4beddcc6e02b..0066a95a6b65 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -67,6 +67,7 @@ import org.apache.cloudstack.api.command.admin.config.ListHypervisorCapabilitiesCmd; import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd; import org.apache.cloudstack.api.command.admin.config.UpdateHypervisorCapabilitiesCmd; +import org.apache.cloudstack.api.command.admin.direct.download.RevokeTemplateDirectDownloadCertificateCmd; import org.apache.cloudstack.api.command.admin.direct.download.UploadTemplateDirectDownloadCertificateCmd; import org.apache.cloudstack.api.command.admin.domain.CreateDomainCmd; import org.apache.cloudstack.api.command.admin.domain.DeleteDomainCmd; @@ -3100,6 +3101,7 @@ public List> getCommands() { cmdList.add(CreateManagementNetworkIpRangeCmd.class); cmdList.add(DeleteManagementNetworkIpRangeCmd.class); cmdList.add(UploadTemplateDirectDownloadCertificateCmd.class); + cmdList.add(RevokeTemplateDirectDownloadCertificateCmd.class); cmdList.add(ListMgmtsCmd.class); cmdList.add(GetUploadParamsForIsoCmd.class); diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 99860934cd56..2eb6d36b9bf9 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -22,9 +22,13 @@ import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; import com.cloud.event.ActionEventUtils; import com.cloud.event.EventTypes; import com.cloud.event.EventVO; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; @@ -37,6 +41,7 @@ import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.exception.CloudRuntimeException; import java.net.URI; @@ -51,8 +56,12 @@ import java.util.Map; import java.util.Arrays; import java.util.Collections; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import javax.inject.Inject; +import javax.naming.ConfigurationException; import com.cloud.utils.security.CertificateHelper; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; @@ -62,18 +71,24 @@ import org.apache.cloudstack.agent.directdownload.MetalinkDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.NfsDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.HttpsDirectDownloadCommand; +import org.apache.cloudstack.agent.directdownload.RevokeDirectDownloadCertificateCommand; import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.poll.BackgroundPollManager; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import sun.security.x509.X509CertImpl; public class DirectDownloadManagerImpl extends ManagerBase implements DirectDownloadManager { @@ -96,6 +111,16 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown private VMTemplatePoolDao vmTemplatePoolDao; @Inject private DataStoreManager dataStoreManager; + @Inject + private DirectDownloadCertificateDao directDownloadCertificateDao; + @Inject + private DirectDownloadCertificateHostMapDao directDownloadCertificateHostMapDao; + @Inject + private BackgroundPollManager backgroundPollManager; + @Inject + private DataCenterDao dataCenterDao; + + protected ScheduledExecutorService executorService; @Override public List> getCommands() { @@ -311,12 +336,8 @@ private DirectDownloadCommand getDirectDownloadCommandFromProtocol(DownloadProto /** * Return the list of running hosts to which upload certificates for Direct Download */ - private List getRunningHostsToUploadCertificate(HypervisorType hypervisorType) { - return hostDao.listAllHostsByType(Host.Type.Routing) - .stream() - .filter(x -> x.getStatus().equals(Status.Up) && - x.getHypervisorType().equals(hypervisorType)) - .collect(Collectors.toList()); + private List getRunningHostsToUploadCertificate(Long zoneId, HypervisorType hypervisorType) { + return hostDao.listAllHostsUpByZoneAndHypervisor(zoneId, hypervisorType); } /** @@ -365,22 +386,42 @@ protected void certificateSanity(String certificatePem) { } @Override - public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor) { + public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor, Long zoneId, Long hostId) { if (alias != null && (alias.equalsIgnoreCase("cloud") || alias.startsWith("cloudca"))) { throw new CloudRuntimeException("Please provide a different alias name for the certificate"); } + List hosts; + DirectDownloadCertificateVO certificateVO; HypervisorType hypervisorType = HypervisorType.getType(hypervisor); - List hosts = getRunningHostsToUploadCertificate(hypervisorType); - String certificatePem = getPretifiedCertificate(certificateCer); - certificateSanity(certificatePem); + if (hostId == null) { + hosts = getRunningHostsToUploadCertificate(zoneId, hypervisorType); + + String certificatePem = getPretifiedCertificate(certificateCer); + certificateSanity(certificatePem); + + certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId); + if (certificateVO != null) { + throw new CloudRuntimeException("Certificate alias " + alias + " has been already created"); + } + certificateVO = new DirectDownloadCertificateVO(alias, certificatePem, hypervisorType, zoneId); + directDownloadCertificateDao.persist(certificateVO); + } else { + HostVO host = hostDao.findById(hostId); + hosts = Collections.singletonList(host); + certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId); + if (certificateVO == null) { + s_logger.info("Certificate must be uploaded on zone " + zoneId); + return false; + } + } - s_logger.info("Attempting to upload certificate: " + alias + " to " + hosts.size() + " hosts"); + s_logger.info("Attempting to upload certificate: " + alias + " to " + hosts.size() + " hosts on zone " + zoneId); int hostCount = 0; if (CollectionUtils.isNotEmpty(hosts)) { for (HostVO host : hosts) { - if (!uploadCertificate(certificatePem, alias, host.getId())) { + if (!uploadCertificate(certificateVO.getId(), host.getId())) { String msg = "Could not upload certificate " + alias + " on host: " + host.getName() + " (" + host.getUuid() + ")"; s_logger.error(msg); throw new CloudRuntimeException(msg); @@ -395,20 +436,177 @@ public boolean uploadCertificateToHosts(String certificateCer, String alias, Str /** * Upload and import certificate to hostId on keystore */ - protected boolean uploadCertificate(String certificate, String certificateName, long hostId) { - s_logger.debug("Uploading certificate: " + certificateName + " to host " + hostId); - SetupDirectDownloadCertificateCommand cmd = new SetupDirectDownloadCertificateCommand(certificate, certificateName); + public boolean uploadCertificate(long certificateId, long hostId) { + DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findById(certificateId); + if (certificateVO == null) { + throw new CloudRuntimeException("Could not find certificate with id " + certificateId + " to upload to host: " + hostId); + } + + String certificate = certificateVO.getCertificate(); + String alias = certificateVO.getAlias(); + + s_logger.debug("Uploading certificate: " + certificateVO.getAlias() + " to host " + hostId); + SetupDirectDownloadCertificateCommand cmd = new SetupDirectDownloadCertificateCommand(certificate, alias); Answer answer = agentManager.easySend(hostId, cmd); if (answer == null || !answer.getResult()) { - String msg = "Certificate " + certificateName + " could not be added to host " + hostId; + String msg = "Certificate " + alias + " could not be added to host " + hostId; if (answer != null) { msg += " due to: " + answer.getDetails(); } s_logger.error(msg); return false; } - s_logger.info("Certificate " + certificateName + " successfully uploaded to host: " + hostId); + + s_logger.info("Certificate " + alias + " successfully uploaded to host: " + hostId); + DirectDownloadCertificateHostMapVO map = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateId, hostId); + if (map != null) { + map.setRevoked(false); + directDownloadCertificateHostMapDao.update(map.getId(), map); + } else { + DirectDownloadCertificateHostMapVO mapVO = new DirectDownloadCertificateHostMapVO(certificateId, hostId); + directDownloadCertificateHostMapDao.persist(mapVO); + } + return true; } + @Override + public boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId, Long hostId) { + HypervisorType hypervisorType = HypervisorType.getType(hypervisor); + DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(certificateAlias, hypervisorType, zoneId); + if (certificateVO == null) { + throw new CloudRuntimeException("Certificate alias " + certificateAlias + " does not exist"); + } + + List maps = null; + if (hostId == null) { + maps = directDownloadCertificateHostMapDao.listByCertificateId(certificateVO.getId()); + } else { + DirectDownloadCertificateHostMapVO hostMap = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateVO.getId(), hostId); + if (hostMap == null) { + s_logger.info("Certificate " + certificateAlias + " cannot be revoked from host " + hostId + " as it is not available on the host"); + return false; + } + maps = Collections.singletonList(hostMap); + } + + s_logger.info("Attempting to revoke certificate alias: " + certificateAlias + " from " + maps.size() + " hosts"); + if (CollectionUtils.isNotEmpty(maps)) { + for (DirectDownloadCertificateHostMapVO map : maps) { + Long mappingHostId = map.getHostId(); + if (!revokeCertificateAliasFromHost(certificateAlias, mappingHostId)) { + String msg = "Could not revoke certificate from host: " + mappingHostId; + s_logger.error(msg); + throw new CloudRuntimeException(msg); + } + s_logger.info("Certificate " + certificateAlias + " revoked from host " + mappingHostId); + map.setRevoked(true); + directDownloadCertificateHostMapDao.update(map.getId(), map); + } + } + return true; + } + + protected boolean revokeCertificateAliasFromHost(String alias, Long hostId) { + RevokeDirectDownloadCertificateCommand cmd = new RevokeDirectDownloadCertificateCommand(alias); + try { + Answer answer = agentManager.send(hostId, cmd); + return answer != null && answer.getResult(); + } catch (AgentUnavailableException | OperationTimedoutException e) { + s_logger.error("Error revoking certificate " + alias + " from host " + hostId, e); + } + return false; + } + + @Override + public boolean configure(String name, Map params) throws ConfigurationException { + executorService = new ScheduledThreadPoolExecutor(1, new NamedThreadFactory("DirectDownloadCertificateMonitor")); + return true; + } + + @Override + public boolean stop() { + executorService.shutdownNow(); + return true; + } + + @Override + public boolean start() { + if (DirectDownloadCertificateUploadInterval.value() > 0L) { + executorService.scheduleWithFixedDelay( + new DirectDownloadCertificateUploadBackgroundTask(this, hostDao, dataCenterDao, + directDownloadCertificateDao, directDownloadCertificateHostMapDao), + 60L, DirectDownloadCertificateUploadInterval.value(), TimeUnit.HOURS); + } + return true; + } + + @Override + public String getConfigComponentName() { + return DirectDownloadManager.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[]{ + DirectDownloadCertificateUploadInterval + }; + } + + public static final class DirectDownloadCertificateUploadBackgroundTask extends ManagedContextRunnable { + + private DirectDownloadManager directDownloadManager; + private HostDao hostDao; + private DirectDownloadCertificateDao directDownloadCertificateDao; + private DirectDownloadCertificateHostMapDao directDownloadCertificateHostMapDao; + private DataCenterDao dataCenterDao; + + public DirectDownloadCertificateUploadBackgroundTask( + final DirectDownloadManager manager, + final HostDao hostDao, + final DataCenterDao dataCenterDao, + final DirectDownloadCertificateDao directDownloadCertificateDao, + final DirectDownloadCertificateHostMapDao directDownloadCertificateHostMapDao) { + this.directDownloadManager = manager; + this.hostDao = hostDao; + this.dataCenterDao = dataCenterDao; + this.directDownloadCertificateDao = directDownloadCertificateDao; + this.directDownloadCertificateHostMapDao = directDownloadCertificateHostMapDao; + } + + @Override + protected void runInContext() { + try { + if (s_logger.isTraceEnabled()) { + s_logger.trace("Direct Download Manager background task is running..."); + } + final DateTime now = DateTime.now(DateTimeZone.UTC); + List enabledZones = dataCenterDao.listEnabledZones(); + for (DataCenterVO zone : enabledZones) { + List zoneCertificates = directDownloadCertificateDao.listByZone(zone.getId()); + if (CollectionUtils.isNotEmpty(zoneCertificates)) { + for (DirectDownloadCertificateVO certificateVO : zoneCertificates) { + List hostsToUpload = hostDao.listAllHostsUpByZoneAndHypervisor(certificateVO.getZoneId(), certificateVO.getHypervisorType()); + if (CollectionUtils.isNotEmpty(hostsToUpload)) { + for (HostVO hostVO : hostsToUpload) { + DirectDownloadCertificateHostMapVO mapping = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateVO.getId(), hostVO.getId()); + if (mapping == null) { + s_logger.debug("Certificate " + certificateVO.getId() + + " (" + certificateVO.getAlias() + ") was not uploaded to host: " + hostVO.getId() + + " uploading it"); + boolean result = directDownloadManager.uploadCertificate(certificateVO.getId(), hostVO.getId()); + s_logger.debug("Certificate " + certificateVO.getAlias() + " " + + (result ? "uploaded" : "could not be uploaded") + + " to host " + hostVO.getId()); + } + } + } + } + } + } + } catch (final Throwable t) { + s_logger.error("Error trying to run Direct Download background task", t); + } + } + } } diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py index 65117f97f8c0..132deb4f6989 100644 --- a/test/integration/smoke/test_direct_download.py +++ b/test/integration/smoke/test_direct_download.py @@ -27,7 +27,7 @@ from marvin.lib.common import (get_pod, get_zone) from nose.plugins.attrib import attr -from marvin.cloudstackAPI import uploadTemplateDirectDownloadCertificate +from marvin.cloudstackAPI import (uploadTemplateDirectDownloadCertificate, revokeTemplateDirectDownloadCertificate) from marvin.lib.decoratorGenerators import skipTestIf @@ -92,6 +92,7 @@ def test_01_sanity_check_on_certificates(self): cmd.hypervisor = self.hypervisor cmd.name = "marvin-test-verify-certs" cmd.certificate = self.certificates["invalid"] + cmd.zoneid = self.zone.id invalid_cert_uploadFails = False expired_cert_upload_fails = False @@ -120,17 +121,29 @@ def test_02_upload_direct_download_certificates(self): # Validate the following # 1. Valid certificates are uploaded to hosts + # 2. Revoke uploaded certificate from host cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() cmd.hypervisor = self.hypervisor cmd.name = "marvin-test-verify-certs" cmd.certificate = self.certificates["valid"] + cmd.zoneid = self.zone.id try: self.apiclient.uploadTemplateDirectDownloadCertificate(cmd) except Exception as e: self.fail("Valid certificate must be uploaded") + revokecmd = revokeTemplateDirectDownloadCertificate.revokeTemplateDirectDownloadCertificateCmd() + revokecmd.hypervisor = self.hypervisor + revokecmd.name = cmd.name + revokecmd.zoneid = self.zone.id + + try: + self.apiclient.revokeTemplateDirectDownloadCertificate(revokecmd) + except Exception as e: + self.fail("Uploaded certificates should be revoked when needed") + return From 5d8157422dbbcacc66543185544c00eb2bb49c3a Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 18 Jul 2019 22:12:55 +0530 Subject: [PATCH 03/10] Allow users to share templates with Accounts or Projects through the UI * Allow users to share templates with Accounts or Projects through the updateTemplate permissions API * Change behaviour to show only supported projects and accounts with update template permissions * Allow admins to see accounts dropdown and only hide lists for users * Don't allow sharing project owned templates as you cannot retrieve them in list api calls --- ...BaseUpdateTemplateOrIsoPermissionsCmd.java | 3 +- .../user/config/ListCapabilitiesCmd.java | 1 + .../api/response/CapabilitiesResponse.java | 8 + .../apache/cloudstack/query/QueryService.java | 4 + .../java/com/cloud/api/ApiResponseHelper.java | 5 + .../com/cloud/api/query/QueryManagerImpl.java | 13 +- .../api/query/dao/UserVmJoinDaoImpl.java | 6 +- .../cloud/server/ManagementServerImpl.java | 7 +- .../cloud/template/TemplateManagerImpl.java | 74 ++-- ui/css/cloudstack3.css | 8 + ui/l10n/en.js | 5 + ui/scripts/cloudStack.js | 2 + ui/scripts/docs.js | 22 +- ui/scripts/sharedFunctions.js | 1 + ui/scripts/templates.js | 319 +++++++++++++++++- 15 files changed, 424 insertions(+), 54 deletions(-) mode change 100755 => 100644 ui/scripts/templates.js diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoPermissionsCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoPermissionsCmd.java index 77e5a15b09e9..410ffefb00dd 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoPermissionsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoPermissionsCmd.java @@ -45,7 +45,7 @@ protected String getResponseName() { @Parameter(name = ApiConstants.ACCOUNTS, type = CommandType.LIST, collectionType = CommandType.STRING, - description = "a comma delimited list of accounts. If specified, \"op\" parameter has to be passed in.") + description = "a comma delimited list of accounts within caller's domain. If specified, \"op\" parameter has to be passed in.") private List accountNames; @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = TemplateResponse.class, required = true, description = "the template ID") @@ -80,7 +80,6 @@ public List getAccountNames() { if (accountNames != null && projectIds != null) { throw new InvalidParameterValueException("Accounts and projectIds can't be specified together"); } - return accountNames; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java index 9c526563d44d..40d1a71e9662 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java @@ -59,6 +59,7 @@ public void execute() { response.setKVMSnapshotEnabled((Boolean)capabilities.get("KVMSnapshotEnabled")); response.setAllowUserViewDestroyedVM((Boolean)capabilities.get("allowUserViewDestroyedVM")); response.setAllowUserExpungeRecoverVM((Boolean)capabilities.get("allowUserExpungeRecoverVM")); + response.setAllowUserViewAllDomainAccounts((Boolean)capabilities.get("allowUserViewAllDomainAccounts")); if (capabilities.containsKey("apiLimitInterval")) { response.setApiLimitInterval((Integer)capabilities.get("apiLimitInterval")); } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java index bcdad468fac8..153d7dfca9ae 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java @@ -84,6 +84,10 @@ public class CapabilitiesResponse extends BaseResponse { @Param(description = "true if the user can recover and expunge virtualmachines, false otherwise", since = "4.6.0") private boolean allowUserExpungeRecoverVM; + @SerializedName("allowuserviewalldomainaccounts") + @Param(description = "true if users can see all accounts within the same domain, false otherwise") + private boolean allowUserViewAllDomainAccounts; + public void setSecurityGroupsEnabled(boolean securityGroupsEnabled) { this.securityGroupsEnabled = securityGroupsEnabled; } @@ -143,4 +147,8 @@ public void setAllowUserViewDestroyedVM(boolean allowUserViewDestroyedVM) { public void setAllowUserExpungeRecoverVM(boolean allowUserExpungeRecoverVM) { this.allowUserExpungeRecoverVM = allowUserExpungeRecoverVM; } + + public void setAllowUserViewAllDomainAccounts(boolean allowUserViewAllDomainAccounts) { + this.allowUserViewAllDomainAccounts = allowUserViewAllDomainAccounts; + } } \ No newline at end of file diff --git a/api/src/main/java/org/apache/cloudstack/query/QueryService.java b/api/src/main/java/org/apache/cloudstack/query/QueryService.java index 618a8f6f8a5a..b9010cb89010 100644 --- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java +++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java @@ -103,6 +103,10 @@ public interface QueryService { "network offering, zones), we use the flag to determine if the entities should be sorted ascending (when flag is true) " + "or descending (when flag is false). Within the scope of the config all users see the same result.", true, ConfigKey.Scope.Global); + public static final ConfigKey AllowUserViewAllDomainAccounts = new ConfigKey<>("Advanced", Boolean.class, + "allow.user.view.all.domain.accounts", "false", + "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain); + ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException; ListResponse searchForEvents(ListEventsCmd cmd); diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 524e109a2b12..20bfb9642d07 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -1808,6 +1808,11 @@ public TemplatePermissionsResponse createTemplatePermissionsResponse(ResponseVie List regularAccounts = new ArrayList(); for (String accountName : accountNames) { Account account = ApiDBUtils.findAccountByNameDomain(accountName, templateOwner.getDomainId()); + if (account == null) { + s_logger.error("Missing Account " + accountName + " in domain " + templateOwner.getDomainId()); + continue; + } + if (account.getType() != Account.ACCOUNT_TYPE_PROJECT) { regularAccounts.add(accountName); } else { diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 92a110b83a9d..ee56cbb1c137 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -394,6 +394,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q * com.cloud.api.query.QueryService#searchForUsers(org.apache.cloudstack * .api.command.admin.user.ListUsersCmd) */ + @Override public ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException { Pair, Integer> result = searchForUsersInternal(cmd); @@ -1980,7 +1981,8 @@ private Pair, Integer> searchForAccountsInternal(ListAccount // if no "id" specified... if (accountId == null) { // listall only has significance if they are an admin - if (listAll && callerIsAdmin) { + boolean isDomainListAllAllowed = AllowUserViewAllDomainAccounts.valueIn(caller.getDomainId()); + if ((listAll && callerIsAdmin) || isDomainListAllAllowed) { // if no domain id specified, use caller's domain if (domainId == null) { domainId = caller.getDomainId(); @@ -2026,6 +2028,7 @@ private Pair, Integer> searchForAccountsInternal(ListAccount sb.and("needsCleanup", sb.entity().isNeedsCleanup(), SearchCriteria.Op.EQ); sb.and("typeNEQ", sb.entity().getType(), SearchCriteria.Op.NEQ); sb.and("idNEQ", sb.entity().getId(), SearchCriteria.Op.NEQ); + sb.and("type2NEQ", sb.entity().getType(), SearchCriteria.Op.NEQ); if (domainId != null && isRecursive) { sb.and("path", sb.entity().getDomainPath(), SearchCriteria.Op.LIKE); @@ -2035,9 +2038,15 @@ private Pair, Integer> searchForAccountsInternal(ListAccount // don't return account of type project to the end user sc.setParameters("typeNEQ", Account.ACCOUNT_TYPE_PROJECT); + // don't return system account... sc.setParameters("idNEQ", Account.ACCOUNT_ID_SYSTEM); + // do not return account of type domain admin to the end user + if (!callerIsAdmin) { + sc.setParameters("type2NEQ", Account.ACCOUNT_TYPE_DOMAIN_ADMIN); + } + if (keyword != null) { SearchCriteria ssc = _accountJoinDao.createSearchCriteria(); ssc.addOr("accountName", SearchCriteria.Op.LIKE, "%" + keyword + "%"); @@ -3836,6 +3845,6 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {AllowUserViewDestroyedVM, UserVMBlacklistedDetails, UserVMReadOnlyUIDetails, SortKeyAscending}; + return new ConfigKey[] {AllowUserViewDestroyedVM, UserVMBlacklistedDetails, UserVMReadOnlyUIDetails, SortKeyAscending, AllowUserViewAllDomainAccounts}; } } diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index 58b167f6ac3b..0e7374369c33 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -37,12 +37,12 @@ import org.apache.cloudstack.api.response.SecurityGroupResponse; import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.query.QueryService; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; import com.cloud.api.ApiDBUtils; import com.cloud.api.ApiResponseHelper; -import com.cloud.api.query.QueryManagerImpl; import com.cloud.api.query.vo.UserVmJoinVO; import com.cloud.gpu.GPU; import com.cloud.service.ServiceOfferingDetailsVO; @@ -315,14 +315,14 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us } // Remove blacklisted settings if user is not admin if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { - String[] userVmSettingsToHide = QueryManagerImpl.UserVMBlacklistedDetails.value().split(","); + String[] userVmSettingsToHide = QueryService.UserVMBlacklistedDetails.value().split(","); for (String key : userVmSettingsToHide) { resourceDetails.remove(key.trim()); } } userVmResponse.setDetails(resourceDetails); if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { - userVmResponse.setReadOnlyUIDetails(QueryManagerImpl.UserVMReadOnlyUIDetails.value()); + userVmResponse.setReadOnlyUIDetails(QueryService.UserVMReadOnlyUIDetails.value()); } } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 0066a95a6b65..305c0f16b29e 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -535,6 +535,7 @@ import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.framework.security.keystore.KeystoreManager; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.resourcedetail.dao.GuestOsDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; @@ -555,7 +556,6 @@ import com.cloud.alert.AlertVO; import com.cloud.alert.dao.AlertDao; import com.cloud.api.ApiDBUtils; -import com.cloud.api.query.QueryManagerImpl; import com.cloud.capacity.Capacity; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; @@ -3486,9 +3486,11 @@ public Map listCapabilities(final ListCapabilitiesCmd cmd) { final Integer apiLimitInterval = Integer.valueOf(_configDao.getValue(Config.ApiLimitInterval.key())); final Integer apiLimitMax = Integer.valueOf(_configDao.getValue(Config.ApiLimitMax.key())); - final boolean allowUserViewDestroyedVM = (QueryManagerImpl.AllowUserViewDestroyedVM.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId())); + final boolean allowUserViewDestroyedVM = (QueryService.AllowUserViewDestroyedVM.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId())); final boolean allowUserExpungeRecoverVM = (UserVmManager.AllowUserExpungeRecoverVm.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId())); + final boolean allowUserViewAllDomainAccounts = (QueryService.AllowUserViewAllDomainAccounts.valueIn(caller.getDomainId())); + // check if region-wide secondary storage is used boolean regionSecondaryEnabled = false; final List imgStores = _imgStoreDao.findRegionImageStores(); @@ -3508,6 +3510,7 @@ public Map listCapabilities(final ListCapabilitiesCmd cmd) { capabilities.put("KVMSnapshotEnabled", KVMSnapshotEnabled); capabilities.put("allowUserViewDestroyedVM", allowUserViewDestroyedVM); capabilities.put("allowUserExpungeRecoverVM", allowUserExpungeRecoverVM); + capabilities.put("allowUserViewAllDomainAccounts", allowUserViewAllDomainAccounts); if (apiLimitEnabled) { capabilities.put("apiLimitInterval", apiLimitInterval); capabilities.put("apiLimitMax", apiLimitMax); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 373735ce2012..8d732cb28447 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -32,28 +32,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.deploy.DeployDestination; -import com.cloud.storage.ImageStoreUploadMonitorImpl; -import com.cloud.utils.StringUtils; -import com.cloud.utils.EncryptionUtil; -import com.cloud.utils.DateUtil; -import com.cloud.utils.Pair; -import com.cloud.utils.EnumUtils; -import com.cloud.vm.VmDetailConstants; -import com.google.common.base.Joiner; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; - -import org.apache.cloudstack.api.command.user.iso.GetUploadParamsForIsoCmd; -import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd; -import org.apache.cloudstack.framework.async.AsyncCallFuture; -import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand; -import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; -import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; -import org.apache.cloudstack.utils.imagestore.ImageStoreUtil; -import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.collections.MapUtils; -import org.apache.log4j.Logger; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListTemplateOrIsoPermissionsCmd; @@ -61,6 +39,7 @@ import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoPermissionsCmd; import org.apache.cloudstack.api.command.user.iso.DeleteIsoCmd; import org.apache.cloudstack.api.command.user.iso.ExtractIsoCmd; +import org.apache.cloudstack.api.command.user.iso.GetUploadParamsForIsoCmd; import org.apache.cloudstack.api.command.user.iso.ListIsoPermissionsCmd; import org.apache.cloudstack.api.command.user.iso.RegisterIsoCmd; import org.apache.cloudstack.api.command.user.iso.UpdateIsoCmd; @@ -69,6 +48,7 @@ import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd; import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd; import org.apache.cloudstack.api.command.user.template.ExtractTemplateCmd; +import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd; import org.apache.cloudstack.api.command.user.template.ListTemplatePermissionsCmd; import org.apache.cloudstack.api.command.user.template.RegisterTemplateCmd; import org.apache.cloudstack.api.command.user.template.UpdateTemplateCmd; @@ -95,6 +75,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; +import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -104,6 +85,9 @@ import org.apache.cloudstack.storage.command.AttachCommand; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.command.DettachCommand; +import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -111,6 +95,12 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; import org.apache.cloudstack.storage.to.TemplateObjectTO; +import org.apache.cloudstack.utils.imagestore.ImageStoreUtil; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; +import org.apache.log4j.Logger; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; @@ -129,6 +119,7 @@ import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.deploy.DeployDestination; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEvent; @@ -148,6 +139,7 @@ import com.cloud.projects.ProjectManager; import com.cloud.storage.DataStoreRole; import com.cloud.storage.GuestOSVO; +import com.cloud.storage.ImageStoreUploadMonitorImpl; import com.cloud.storage.LaunchPermissionVO; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; @@ -186,6 +178,11 @@ import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.AccountDao; import com.cloud.uservm.UserVm; +import com.cloud.utils.DateUtil; +import com.cloud.utils.EncryptionUtil; +import com.cloud.utils.EnumUtils; +import com.cloud.utils.Pair; +import com.cloud.utils.StringUtils; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.concurrency.NamedThreadFactory; @@ -200,11 +197,12 @@ import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineProfile; +import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; - -import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; +import com.google.common.base.Joiner; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; public class TemplateManagerImpl extends ManagerBase implements TemplateManager, TemplateApiService, Configurable { private final static Logger s_logger = Logger.getLogger(TemplateManagerImpl.class); @@ -1483,9 +1481,24 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions throw new InvalidParameterValueException("unable to update permissions for " + mediaType + " with id " + id); } - boolean isAdmin = _accountMgr.isAdmin(caller.getId()); + Long ownerId = template.getAccountId(); + Account owner = _accountMgr.getAccount(ownerId); + if (ownerId == null) { + // if there is no owner of the template then it's probably already a + // public template (or domain private template) so + // publishing to individual users is irrelevant + throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName()); + } + + if (owner.getType() == Account.ACCOUNT_TYPE_PROJECT) { + // Currently project owned templates cannot be shared outside project but is available to all users within project by default. + throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName() + + ". Project owned templates cannot be shared outside template."); + } + // check configuration parameter(allow.public.user.templates) value for // the template owner + boolean isAdmin = _accountMgr.isAdmin(caller.getId()); boolean allowPublicUserTemplates = AllowPublicUserTemplates.valueIn(template.getAccountId()); if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) { throw new InvalidParameterValueException("Only private " + mediaType + "s can be created."); @@ -1499,14 +1512,6 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions } } - Long ownerId = template.getAccountId(); - if (ownerId == null) { - // if there is no owner of the template then it's probably already a - // public template (or domain private template) so - // publishing to individual users is irrelevant - throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName()); - } - //Only admin or owner of the template should be able to change its permissions if (caller.getId() != ownerId && !isAdmin) { throw new InvalidParameterValueException("Unable to grant permission to account " + caller.getAccountName() + " as it is neither admin nor owner or the template"); @@ -1540,7 +1545,6 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions } //Derive the domain id from the template owner as updateTemplatePermissions is not cross domain operation - Account owner = _accountMgr.getAccount(ownerId); final Domain domain = _domainDao.findById(owner.getDomainId()); if ("add".equalsIgnoreCase(operation)) { final List accountNamesFinal = accountNames; diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 76ba97c038fd..a40e252474c7 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -12421,6 +12421,14 @@ div.ui-dialog div.autoscaler div.field-group div.form-container form div.form-it background-position: -35px -707px; } +.shareTemplate .icon { + background-position: -165px -122px; +} + +.shareTemplate:hover .icon { + background-position: -165px -704px; +} + .createVolume .icon { background-position: -70px -124px; } diff --git a/ui/l10n/en.js b/ui/l10n/en.js index 6a4bba9e3a3f..53e9814fd4d3 100644 --- a/ui/l10n/en.js +++ b/ui/l10n/en.js @@ -92,6 +92,7 @@ var dictionary = { "label.about.app":"About CloudStack", "label.accept.project.invitation":"Accept project invitation", "label.account":"Account", +"label.accounts":"Accounts", "label.account.and.security.group":"Account, Security group", "label.account.details":"Account details", "label.account.id":"Account ID", @@ -279,6 +280,7 @@ var dictionary = { "label.action.run.diagnostics":"Run Diagnostics", "label.action.secure.host":"Provision Host Security Keys", "label.action.start.instance":"Start Instance", +"label.action.share.template": "Update Template Permissions", "label.action.start.instance.processing":"Starting Instance....", "label.action.start.router":"Start Router", "label.action.start.router.processing":"Starting Router....", @@ -1253,6 +1255,7 @@ var dictionary = { "label.opendaylight.controller":"OpenDaylight Controller", "label.opendaylight.controllerdetail":"OpenDaylight Controller Details", "label.opendaylight.controllers":"OpenDaylight Controllers", +"label.operation": "Operation", "label.operator":"Operator", "label.optional":"Optional", "label.order":"Order", @@ -1342,6 +1345,7 @@ var dictionary = { "label.project":"Project", "label.project.dashboard":"Project dashboard", "label.project.id":"Project ID", +"label.project.ids":"Project IDs", "label.project.invite":"Invite to project", "label.project.name":"Project name", "label.project.view":"Project View", @@ -1576,6 +1580,7 @@ var dictionary = { "label.setup.network":"Set up Network", "label.setup.zone":"Set up Zone", "label.shared":"Shared", +"label.share.with":"Share With", "label.show.advanced.settings":"Show advanced settings", "label.show.ingress.rule":"Show Ingress Rule", "label.shutdown.provider":"Shutdown provider", diff --git a/ui/scripts/cloudStack.js b/ui/scripts/cloudStack.js index 8785cd19736f..5280e7e6ff09 100644 --- a/ui/scripts/cloudStack.js +++ b/ui/scripts/cloudStack.js @@ -151,6 +151,8 @@ g_userProjectsEnabled = json.listcapabilitiesresponse.capability.allowusercreateprojects; g_cloudstackversion = json.listcapabilitiesresponse.capability.cloudstackversion; + // Allow users to see all accounts within a domain + g_allowUserViewAllDomainAccounts = json.listcapabilitiesresponse.capability.allowuserviewalldomainaccounts; if (json.listcapabilitiesresponse.capability.apilimitinterval != null && json.listcapabilitiesresponse.capability.apilimitmax != null) { var intervalLimit = ((json.listcapabilitiesresponse.capability.apilimitinterval * 1000) / json.listcapabilitiesresponse.capability.apilimitmax) * 3; //multiply 3 to be on safe side diff --git a/ui/scripts/docs.js b/ui/scripts/docs.js index ec0b32a15aa6..4d00c835a20d 100755 --- a/ui/scripts/docs.js +++ b/ui/scripts/docs.js @@ -1368,29 +1368,41 @@ cloudStack.docs = { desc: 'Pass user and meta data to VMs (via ConfigDrive)', externalLink: '' }, - helpComputeOfferingMinCPUCores: { desc: 'This will be used for the setting the range (min-max) of the number of cpu cores that should be allowed for VMs using this custom offering.', externalLink: '' }, - helpComputeOfferingMaxCPUCores: { desc: 'This will be used for the setting the range (min-max) of the number of cpu cores that should be allowed for VMs using this custom offering.', externalLink: '' }, - helpComputeOfferingMinMemory: { desc: 'This will be used for the setting the range (min-max) amount of memory that should be allowed for VMs using this custom offering.', externalLink: '' }, - helpComputeOfferingMaxMemory: { desc: 'This will be used for the setting the range (min-max) amount of memory that should be allowed for VMs using this custom offering.', externalLink: '' }, - helpComputeOfferingType: { desc: 'This will be used for setting the type of compute offering - whether it is fixed, custom constrained or custom unconstrained.', externalLink: '' + }, + + // Update Template Permissions Helper + helpUpdateTemplateOperation: { + desc: 'Select the permission operator. Add is for sharing with user/project and Reset simply removes all the accounts and projects which template has been shared with.' + }, + helpUpdateTemplateShareWith: { + desc: 'Select account or project with which template is to be shared with.' + }, + helpUpdateTemplateAccounts: { + desc: 'Choose one or more accounts to share this template. Ctrl+Click to select multiple accounts to share with. Selecting "Add > Accounts" shows list of accounts that do not have permissions. Selecting "Remove > Accounts" shows list of accounts that already have permissions.' + }, + helpUpdateTemplateProjectIds: { + desc: 'Choose one or more projects to share this template. Ctrl+Click to select multiple projects to share with. Selecting "Add > Projects" shows list of projects that do not have permissions. Selecting "Remove > Projects" shows list of projects that already have permissions.' + }, + helpUpdateTemplateAccountList: { + desc: 'A comma seperated list of accounts to share the template with. Must be specified with the Add/Remove operation, leave Project ID blank if this is specified.' } }; diff --git a/ui/scripts/sharedFunctions.js b/ui/scripts/sharedFunctions.js index 9fe515153389..84e233f809cc 100644 --- a/ui/scripts/sharedFunctions.js +++ b/ui/scripts/sharedFunctions.js @@ -36,6 +36,7 @@ var g_queryAsyncJobResultInterval = 3000; var g_idpList = null; var g_appendIdpDomain = false; var g_sortKeyIsAscending = false; +var g_allowUserViewAllDomainAccounts= false; //keyboard keycode var keycode_Enter = 13; diff --git a/ui/scripts/templates.js b/ui/scripts/templates.js old mode 100755 new mode 100644 index c64efc973d83..a05e001ed32e --- a/ui/scripts/templates.js +++ b/ui/scripts/templates.js @@ -1507,8 +1507,316 @@ notification: { poll: pollAsyncJobResult } - } + }, + // Share template + shareTemplate: { + label: 'label.action.share.template', + messages: { + notification: function (args) { + return 'label.action.share.template'; + } + }, + + createForm: { + title: 'label.action.share.template', + desc: '', + fields: { + operation: { + label: 'label.operation', + docID: 'helpUpdateTemplateOperation', + validation: { + required: true + }, + select: function (args) { + var items = []; + items.push({ + id: "add", + description: "Add" + }); + items.push({ + id: "remove", + description: "Remove" + }); + items.push({ + id: "reset", + description: "Reset" + }); + + args.response.success({ + data: items + }); + + // Select change + args.$select.change(function () { + var $form = $(this).closest('form'); + var selectedOperation = $(this).val(); + if (selectedOperation === "reset") { + $form.find('[rel=projects]').hide(); + $form.find('[rel=sharewith]').hide(); + $form.find('[rel=accounts]').hide(); + $form.find('[rel=accountlist]').hide(); + } else { + // allow.user.view.domain.accounts = true + // Populate List of accounts in domain as dropdown multiselect + $form.find('[rel=sharewith]').css('display', 'inline-block'); + if (!isUser() || g_allowUserViewAllDomainAccounts === true) { + $form.find('[rel=projects]').css('display', 'inline-block'); + $form.find('[rel=accounts]').css('display', 'inline-block'); + $form.find('[rel=accountlist]').hide(); + } else { + // If users are not allowed to see accounts in the domain, show input text field for Accounts + // Projects will always be shown as dropdown multiselect + $form.find('[rel=projects]').css('display', 'inline-block'); + $form.find('[rel=accountslist]').css('display', 'inline-block'); + $form.find('[rel=accounts]').hide(); + } + } + }); + } + }, + shareWith: { + label: 'label.share.with', + docID: 'helpUpdateTemplateShareWith', + validation: { + required: true + }, + dependsOn: 'operation', + select: function (args) { + var items = []; + items.push({ + id: "account", + description: "Account" + }); + items.push({ + id: "project", + description: "Project" + }); + + args.response.success({ data: items }); + + // Select change + args.$select.change(function () { + var $form = $(this).closest('form'); + var sharedWith = $(this).val(); + if (args.operation !== "reset") { + if (sharedWith === "project") { + $form.find('[rel=accounts]').hide(); + $form.find('[rel=accountlist]').hide(); + $form.find('[rel=projects]').css('display', 'inline-block'); + } else { + // allow.user.view.domain.accounts = true + // Populate List of accounts in domain as dropdown multiselect + if (!isUser() || g_allowUserViewAllDomainAccounts === true) { + $form.find('[rel=projects]').hide(); + $form.find('[rel=accountlist]').hide(); + $form.find('[rel=accounts]').css('display', 'inline-block'); + } else { + // If users are not allowed to see accounts in the domain, show input text field for Accounts + // Projects will always be shown as dropdown multiselect + $form.find('[rel=projects]').hide(); + $form.find('[rel=accounts]').hide(); + $form.find('[rel=accountlist]').css('display', 'inline-block'); + } + } + } + }); + } + }, + + accountlist: { + label: 'label.accounts', + docID: 'helpUpdateTemplateAccountList' + }, + + accounts: { + label: 'label.accounts', + docID: 'helpUpdateTemplateAccounts', + dependsOn: 'shareWith', + isMultiple: true, + select: function (args) { + var operation = args.operation; + if (operation !== "reset") { + $.ajax({ + url: createURL("listAccounts&listall=true"), + dataType: "json", + async: true, + success: function (jsonAccounts) { + var accountByName = {}; + $.each(jsonAccounts.listaccountsresponse.account, function(idx, account) { + // Only add current domain's accounts as update template permissions supports that + if (account.domainid === g_domainid) { + accountByName[account.name] = { + projName: account.name, + hasPermission: false + }; + } + }); + $.ajax({ + url: createURL('listTemplatePermissions&id=' + args.context.templates[0].id), + dataType: "json", + async: true, + success: function (json) { + items = json.listtemplatepermissionsresponse.templatepermission.account; + $.each(items, function(idx, accountName) { + accountByName[accountName].hasPermission = true; + }); + + var accountObjs = []; + if (operation === "add") { + // Skip already permitted accounts + $.each(Object.keys(accountByName), function(idx, accountName) { + if (accountByName[accountName].hasPermission == false) { + accountObjs.push({ + name: accountName, + description: accountName + }); + } + }); + } else if (items != null) { + $.each(items, function(idx, accountName) { + if (accountName !== g_account) { + accountObjs.push({ + name: accountName, + description: accountName + }); + } + }); + } + args.$select.html(''); + args.response.success({data: accountObjs}); + } + }); + } + }); + } + } + }, + + projects: { + label: 'label.projects', + docID: 'helpUpdateTemplateProjectIds', + dependsOn: 'shareWith', + isMultiple: true, + select: function (args) { + var operation = args.operation; + if (operation !== "reset") { + $.ajax({ + url: createURL("listProjects&listall=true"), + dataType: "json", + async: true, + success: function (jsonProjects) { + var projectsByIds = {}; + $.each(jsonProjects.listprojectsresponse.project, function(idx, project) { + // Only add current domain's projects as update template permissions supports that + if (project.domainid === g_domainid) { + projectsByIds[project.id] = { + projName: project.name, + hasPermission: false + }; + } + }); + + $.ajax({ + url: createURL('listTemplatePermissions&id=' + args.context.templates[0].id), + dataType: "json", + async: true, + success: function (json) { + items = json.listtemplatepermissionsresponse.templatepermission.projectids; + $.each(items, function(idx, projectId) { + projectsByIds[projectId].hasPermission = true; + }); + var projectObjs = []; + if (operation === "add") { + // Skip already permitted accounts + $.each(Object.keys(projectsByIds), function(idx, projectId) { + if (projectsByIds[projectId].hasPermission == false) { + projectObjs.push({ + id: projectId, + description: projectsByIds[projectId].projName + }); + } + }); + } else if (items != null) { + $.each(items, function(idx, projectId) { + if (projectId !== g_account) { + projectObjs.push({ + id: projectId, + description: projectsByIds[projectId] ? projectsByIds[projectId].projName : projectId + }); + } + }); + } + args.$select.html(''); + args.response.success({data: projectObjs}); + } + }); + } + }); + } + } + } + } + }, + + action: function (args) { + // Load data from form + var data = { + id: args.context.templates[0].id, + op: args.data.operation + }; + var selectedOperation = args.data.operation; + if (selectedOperation === "reset") { + // Do not append Project ID or Account to data object + } else { + var projects = args.data.projects; + var accounts = args.data.accounts; + var accountList = args.data.accountlist; + + if (accounts !== undefined || (accountList !== undefined && accountList.length > 0)) { + var accountNames = ""; + if (accountList !== undefined && accounts === undefined) { + accountNames = accountList; + } else { + if (Object.prototype.toString.call(accounts) === '[object Array]') { + accountNames = accounts.join(","); + } else { + accountNames = accounts; + } + } + $.extend(data, { + accounts: accountNames + }); + } + + if (projects !== undefined) { + var projectIds = ""; + if (Object.prototype.toString.call(projects) === '[object Array]') { + projectIds = projects.join(","); + } else { + projectIds = projects; + } + + $.extend(data, { + projectids: projectIds + }); + } + } + + $.ajax({ + url: createURL('updateTemplatePermissions'), + data: data, + dataType: "json", + async: false, + success: function (json) { + var item = json.updatetemplatepermissionsresponse.success; + args.response.success({ + data: item + }); + } + }); //end ajax + } + } }, tabs: { details: { @@ -1882,11 +2190,11 @@ }else if(args.page == 1) { args.response.success({ data: [] - }); + }); } else { args.response.success({ data: [] - }); + }); } } }); @@ -2202,7 +2510,7 @@ } } newDetails += 'details[0].' + data.name + '=' + data.value; - + $.ajax({ url: createURL('updateTemplate&id=' + args.context.templates[0].id + '&' + newDetails), success: function(json) { @@ -3429,7 +3737,7 @@ allowedActions.push("copyTemplate"); } - // "Download Template" + // "Download Template" , "Update Template Permissions" if (((isAdmin() == false && !(jsonObj.domainid == g_domainid && jsonObj.account == g_account) && !(jsonObj.domainid == g_domainid && cloudStack.context.projects && jsonObj.projectid == cloudStack.context.projects[0].id))) //if neither root-admin, nor the same account, nor the same project || (jsonObj.isready == false) || jsonObj.templatetype == "SYSTEM") { //do nothing @@ -3437,6 +3745,7 @@ if (jsonObj.isextractable){ allowedActions.push("downloadTemplate"); } + allowedActions.push("shareTemplate"); } // "Delete Template" From 6a336f8bc11b80846ed86e188fa6e5da18524d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Beims=20Br=C3=A4scher?= Date: Mon, 22 Jul 2019 08:52:02 -0300 Subject: [PATCH 04/10] server: disable unauthenticated integration.api.port by default (#3504) Set integration.api.port to (0) zero as default. CloudStack provides CloudStack API Unauthenticated Access through port 8096. It should not be open to the Internet in any case. --- server/src/main/java/com/cloud/api/ApiServer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index a8ab7b095c67..cc9ec73c94e9 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -228,8 +228,8 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private static final ConfigKey IntegrationAPIPort = new ConfigKey("Advanced" , Integer.class , "integration.api.port" - , "8096" - , "Default API port" + , "0" + , "Integration (unauthenticated) API port. To disable set it to 0 or negative." , false , ConfigKey.Scope.Global); private static final ConfigKey ConcurrentSnapshotsThresholdPerHost = new ConfigKey("Advanced" From e1fa270593bb49ccbc85ffa2315570b8d7d8b2e1 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Mon, 22 Jul 2019 17:27:41 +0530 Subject: [PATCH 05/10] vmware: fix volume stats logic (#3473) During volume stats calculation, if a volume has more than one disk in the chain-info it is not used to sum the physical and virtual size in the loop, instead any previous entry was overwritten by the last disk. Signed-off-by: Rohit Yadav --- .../hypervisor/vmware/resource/VmwareResource.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index c195712e62ac..141f2f635558 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -3612,8 +3612,16 @@ protected GetVolumeStatsAnswer execute(GetVolumeStatsCommand cmd) { Pair vds = vmMo.getDiskDevice(file.getFileName(), true); long virtualsize = vds.first().getCapacityInKB() * 1024; long physicalsize = primaryStorageDatastoreMo.fileDiskSize(file.getPath()); - VolumeStatsEntry vse = new VolumeStatsEntry(chainInfo, physicalsize, virtualsize); - statEntry.put(chainInfo, vse); + if (statEntry.containsKey(chainInfo)) { + VolumeStatsEntry vse = statEntry.get(chainInfo); + if (vse != null) { + vse.setPhysicalSize(vse.getPhysicalSize() + physicalsize); + vse.setVirtualSize(vse.getVirtualSize() + virtualsize); + } + } else { + VolumeStatsEntry vse = new VolumeStatsEntry(chainInfo, physicalsize, virtualsize); + statEntry.put(chainInfo, vse); + } } } } From 38f97c65b81b5b99d6dfef7641e679f79b42deed Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 22 Jul 2019 09:03:00 -0300 Subject: [PATCH 06/10] server: Fix hardcoded max data volumes when VM has been created but not started before (#3494) When VM is created but not started on any host, attaching a volume on it causes the maximum number of allowed volumes to be 6 (hardcoded). --- .../hypervisor/dao/HypervisorCapabilitiesDao.java | 2 ++ .../dao/HypervisorCapabilitiesDaoImpl.java | 13 +++++++++++++ .../com/cloud/storage/VolumeApiServiceImpl.java | 12 +++++++++++- .../com/cloud/storage/VolumeApiServiceImplTest.java | 3 +++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java index 029a8fa273cf..83c32b1c2efd 100644 --- a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java +++ b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDao.java @@ -35,4 +35,6 @@ public interface HypervisorCapabilitiesDao extends GenericDao getHypervisorsWithDefaultEntries(); } diff --git a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java index a4341380ddc4..5cecff2af95f 100644 --- a/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.hypervisor.dao; +import java.util.ArrayList; import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -106,4 +107,16 @@ public Boolean isVmSnapshotEnabled(HypervisorType hypervisorType, String hypervi HypervisorCapabilitiesVO result = getCapabilities(hypervisorType, hypervisorVersion); return result.getVmSnapshotEnabled(); } + + @Override + public List getHypervisorsWithDefaultEntries() { + SearchCriteria sc = HypervisorTypeAndVersionSearch.create(); + sc.setParameters("hypervisorVersion", DEFAULT_VERSION); + List hypervisorCapabilitiesVOS = listBy(sc); + List hvs = new ArrayList<>(); + for (HypervisorCapabilitiesVO capabilitiesVO : hypervisorCapabilitiesVOS) { + hvs.add(capabilitiesVO.getHypervisorType()); + } + return hvs; + } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 5f3ab4c81168..2a3d39578289 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -266,6 +266,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic private List _storagePoolAllocators; + private List supportingDefaultHV; + VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this); static final ConfigKey VmJobCheckInterval = new ConfigKey("Advanced", Long.class, "vm.job.check.interval", "3000", "Interval in milliseconds to check if the job is complete", false); @@ -2857,7 +2859,9 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L } } - verifyManagedStorage(volumeToAttachStoragePool.getId(), hostId); + if (volumeToAttachStoragePool != null) { + verifyManagedStorage(volumeToAttachStoragePool.getId(), hostId); + } // volumeToAttachStoragePool should be null if the VM we are attaching the disk to has never been started before DataStore dataStore = volumeToAttachStoragePool != null ? dataStoreMgr.getDataStore(volumeToAttachStoragePool.getId(), DataStoreRole.Primary) : null; @@ -3003,6 +3007,11 @@ private int getMaxDataVolumesSupported(UserVmVO vm) { if (host != null) { _hostDao.loadDetails(host); maxDataVolumesSupported = _hypervisorCapabilitiesDao.getMaxDataVolumesLimit(host.getHypervisorType(), host.getDetail("product_version")); + } else { + HypervisorType hypervisorType = vm.getHypervisorType(); + if (hypervisorType != null && CollectionUtils.isNotEmpty(supportingDefaultHV) && supportingDefaultHV.contains(hypervisorType)) { + maxDataVolumesSupported = _hypervisorCapabilitiesDao.getMaxDataVolumesLimit(hypervisorType, "default"); + } } if (maxDataVolumesSupported == null || maxDataVolumesSupported.intValue() <= 0) { maxDataVolumesSupported = 6; // 6 data disks by default if nothing @@ -3050,6 +3059,7 @@ private Long getDeviceId(UserVmVO vm, Long deviceId) { public boolean configure(String name, Map params) { String maxVolumeSizeInGbString = _configDao.getValue(Config.MaxVolumeSize.toString()); _maxVolumeSizeInGb = NumbersUtil.parseLong(maxVolumeSizeInGbString, 2000); + supportingDefaultHV = _hypervisorCapabilitiesDao.getHypervisorsWithDefaultEntries(); return true; } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 9d1426ce9f15..da346536d5a3 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -33,6 +33,7 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; +import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd; @@ -149,6 +150,8 @@ public class VolumeApiServiceImplTest { private HostDao _hostDao; @Mock private StoragePoolTagsDao storagePoolTagsDao; + @Mock + private HypervisorCapabilitiesDao hypervisorCapabilitiesDao; private DetachVolumeCmd detachCmd = new DetachVolumeCmd(); private Class _detachCmdClass = detachCmd.getClass(); From 5690cd636427eb70ea09a732bf75163ec77af15e Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Tue, 23 Jul 2019 00:27:50 +0530 Subject: [PATCH 07/10] server: fix the subnet overlap checking logic for tagged and untagged vlans when adding ipranges (#3430) Fixes: #3114 When adding iprange for VLANs there are 3 cases - VLAN under consideration has a tag (like 101) VLAN under consideration has a tag but as a range (like 101-124) VLAN is untagged (i.e. id is "untagged") Before adding iprange we have to check for possible overlaps and throw exception. This needs to be done as follows - If VLAN Tag ID is numeric or a range we need to call UriUtils.checkVlanUriOverlapmethod which internally tries to expand the range as verifies if there are overlaps. If URI overlaps (i.e. there are overlapping VLAN tags) we then need to verify if the iprange being added overlaps with previously added ranges. If there are no overlapping tags we simply need to test for public networks being present in the VLAN. A Regression was introduced in 41fdb88#diff-6e2b61984e8fa2823bb47da3caafa4eeR3174 which caused comparing 'untagged' string as a numeric VLAN Tag range and and attempted expanding it to test overlap in UriUtils.checkVlanUriOverlap. To fix the bug in the issue, we need to handle the untagged case separately as it's non-numeric tag in code. For untagged VLANs and overlapping VLAN URIs we need to check for ipranges and gateways which happens naturally after this change. For tagged VLANs with non-overlapping URIs we need to check if there is a public network. --- .../ConfigurationManagerImpl.java | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index bf238b0a7255..2c71f0db11c8 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -3815,29 +3815,10 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId, continue; } // from here, subnet overlaps - if (!UriUtils.checkVlanUriOverlap( + if (vlanId.toLowerCase().contains(Vlan.UNTAGGED) || UriUtils.checkVlanUriOverlap( BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId)), BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlan.getVlanTag())))) { - boolean overlapped = false; - if( network.getTrafficType() == TrafficType.Public ) { - overlapped = true; - } else { - final Long nwId = vlan.getNetworkId(); - if ( nwId != null ) { - final Network nw = _networkModel.getNetwork(nwId); - if ( nw != null && nw.getTrafficType() == TrafficType.Public ) { - overlapped = true; - } - } - - } - if ( overlapped ) { - throw new InvalidParameterValueException("The IP range with tag: " + vlan.getVlanTag() - + " in zone " + zone.getName() - + " has overlapped with the subnet. Please specify a different gateway/netmask."); - } - } else { - + // For untagged VLAN Id and overlapping URIs we need to expand and verify IP ranges final String[] otherVlanIpRange = vlan.getIpRange().split("\\-"); final String otherVlanStartIP = otherVlanIpRange[0]; String otherVlanEndIP = null; @@ -3854,9 +3835,29 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId, if (!NetUtils.is31PrefixCidr(newCidr)) { if (NetUtils.ipRangesOverlap(startIP, endIP, otherVlanStartIP, otherVlanEndIP)) { throw new InvalidParameterValueException("The IP range already has IPs that overlap with the new range." + - " Please specify a different start IP/end IP."); + " Please specify a different start IP/end IP."); } } + } else { + // For tagged or non-overlapping URIs we need to ensure there is no Public traffic type + boolean overlapped = false; + if (network.getTrafficType() == TrafficType.Public) { + overlapped = true; + } else { + final Long nwId = vlan.getNetworkId(); + if (nwId != null) { + final Network nw = _networkModel.getNetwork(nwId); + if (nw != null && nw.getTrafficType() == TrafficType.Public) { + overlapped = true; + } + } + + } + if (overlapped) { + throw new InvalidParameterValueException("The IP range with tag: " + vlan.getVlanTag() + + " in zone " + zone.getName() + + " has overlapped with the subnet. Please specify a different gateway/netmask."); + } } } } From 1c8bc3b7051c145c42476dc88fe4a23fd0ca1f84 Mon Sep 17 00:00:00 2001 From: Dingane Hlaluku Date: Tue, 28 Aug 2018 16:13:22 +0200 Subject: [PATCH 08/10] Suqash commits to a single commit and rebase against master Update marvin tests to use white list --- .../cloud/vm/VirtualMachineManagerImpl.java | 13 + .../resource/LibvirtComputingResource.java | 6 +- .../resource/CitrixResourceBase.java | 12 +- .../xenserver/ExtraConfigurationUtility.java | 170 +++++++ .../java/com/cloud/vm/UserVmManagerImpl.java | 218 ++++++++- .../smoke/test_deploy_vm_extra_config_data.py | 453 ++++++++++++++++++ 6 files changed, 845 insertions(+), 27 deletions(-) create mode 100644 plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java create mode 100644 test/integration/smoke/test_deploy_vm_extra_config_data.py diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 1cc925b3ccf5..e9a9f074f906 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1119,6 +1119,9 @@ public void orchestrateStart(final String vmUuid, final Map details = vmTO.getDetails(); + for (String key : details.keySet()) { + if (key.startsWith(ApiConstants.EXTRA_CONFIG)) { + vmTO.addExtraConfig(key, details.get(key)); + } + } + } + // for managed storage on KVM, need to make sure the path field of the volume in question is populated with the IQN private void handlePath(final DiskTO[] disks, final HypervisorType hypervisorType) { if (hypervisorType != HypervisorType.KVM) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index b20f1a58b949..df187395b211 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2223,7 +2223,11 @@ So if getMinSpeed() returns null we fall back to getSpeed(). vm.addComp(devices); - addExtraConfigComponent(extraConfig, vm); + // Add extra configuration to User VM Domain XML before starting + if (vmTO.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) { + s_logger.info("Appending extra configuration data to guest VM domain XML"); + addExtraConfigComponent(extraConfig, vm); + } return vm; } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 79a9fb229724..4117892b59c2 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -49,6 +49,7 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.hypervisor.xenserver.ExtraConfigurationUtility; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; @@ -1404,7 +1405,7 @@ public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmS } } try { - finalizeVmMetaData(vm, conn, vmSpec); + finalizeVmMetaData(vm, vmr, conn, vmSpec); } catch (final Exception e) { throw new CloudRuntimeException("Unable to finalize VM MetaData: " + vmSpec); } @@ -1859,7 +1860,7 @@ protected void fillHostInfo(final Connection conn, final StartupRoutingCommand c } } - protected void finalizeVmMetaData(final VM vm, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { + protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { final Map details = vmSpec.getDetails(); if (details != null) { @@ -1890,6 +1891,13 @@ protected void finalizeVmMetaData(final VM vm, final Connection conn, final Virt } } } + + // Add configuration settings VM record for User VM instances before creating VM + Map extraConfig = vmSpec.getExtraConfig(); + if (vmSpec.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) { + s_logger.info("Appending user extra configuration settings to VM"); + ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig); + } } /** diff --git a/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java new file mode 100644 index 000000000000..597d65bbebfc --- /dev/null +++ b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java @@ -0,0 +1,170 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.hypervisor.xenserver; + +import java.util.HashMap; +import java.util.Map; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.exception.CloudRuntimeException; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Types; +import com.xensource.xenapi.VM; +import org.apache.log4j.Logger; +import org.apache.xmlrpc.XmlRpcException; + +public class ExtraConfigurationUtility { + private static final Logger LOG = Logger.getLogger(ExtraConfigurationUtility.class); + + public static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM vm, Map extraConfig) { + Map recordMap = vmr.toMap(); + for (String key : extraConfig.keySet()) { + String cfg = extraConfig.get(key); + Map configParams = prepareKeyValuePair(cfg); + + // paramKey is either param or param:key for map parameters + String paramKey = configParams.keySet().toString().replaceAll("[\\[\\]]", ""); + String paramValue = configParams.get(paramKey); + + //Map params + if (paramKey.contains(":")) { + int i = paramKey.indexOf(":"); + String actualParam = paramKey.substring(0, i); + String keyName = paramKey.substring(i + 1); + boolean isValidOperation = isValidOperation(recordMap, actualParam); + if (isValidOperation) { + try { + switch (actualParam) { + case "VCPUs_params": + vm.addToVCPUsParams(conn, keyName, paramValue); + break; + case "platform": + vm.addToOtherConfig(conn, keyName, paramValue); + break; + case "HVM_boot_params": + vm.addToHVMBootParams(conn, keyName, paramValue); + break; + case "other_config": + vm.addToOtherConfig(conn, keyName, paramValue); + break; + case "xenstore_data": + vm.addToXenstoreData(conn, keyName, paramValue); + break; + default: + String msg = String.format("Passed configuration %s is not supported", paramKey); + LOG.warn(msg); + } + } catch (XmlRpcException | Types.XenAPIException e) { + LOG.error("Exception caught while setting VM configuration: " + cfg); + throw new CloudRuntimeException("Exception caught while setting VM configuration: " + cfg, e); + } + } else { + LOG.error("Unsupported extra configuration has been passed"); + throw new InvalidParameterValueException("Unsupported extra configuration option has been passed: " + actualParam); + } + + } else { + boolean isValidOperation = isValidOperation(recordMap, paramKey); + if (isValidOperation) { + try { + switch (paramKey) { + case "HVM_boot_policy": + vm.setHVMBootPolicy(conn, paramValue); + break; + case "HVM_shadow_multiplier": + vm.setHVMShadowMultiplier(conn, Double.valueOf(paramValue)); + break; + case "PV_kernel": + vm.setPVKernel(conn, paramValue); + break; + case "PV_ramdisk": + vm.setPVRamdisk(conn, paramValue); + break; + case "PV_args": + vm.setPVArgs(conn, paramValue); + break; + case "PV_legacy_args": + vm.setPVLegacyArgs(conn, paramValue); + break; + case "PV_bootloader": + vm.setPVBootloader(conn, paramValue); + break; + case "PV_bootloader_args": + vm.setPVBootloaderArgs(conn, paramValue); + break; + case "ha_restart_priority": + vm.setHaRestartPriority(conn, paramValue); + break; + case "start_delay": + vm.setStartDelay(conn, Long.valueOf(paramValue)); + break; + case "shutdown_delay": + vm.setShutdownDelay(conn, Long.valueOf(paramValue)); + break; + case "order": + vm.setOrder(conn, Long.valueOf(paramValue)); + break; + case "VCPUs_max": + vm.setVCPUsMax(conn, Long.valueOf(paramValue)); + break; + case "VCPUs_at_startup": + vm.setVCPUsAtStartup(conn, Long.valueOf(paramValue)); + break; + case "is-a-template": + vm.setIsATemplate(conn, Boolean.valueOf(paramValue)); + break; + case "memory_static_max": + vm.setMemoryStaticMax(conn, Long.valueOf(paramValue)); + break; + case "memory_static_min": + vm.setMemoryStaticMin(conn, Long.valueOf(paramValue)); + break; + case "memory_dynamic_max": + vm.setMemoryDynamicMax(conn, Long.valueOf(paramValue)); + break; + case "memory_dynamic_min": + vm.setMemoryDynamicMin(conn, Long.valueOf(paramValue)); + break; + default: + String anotherMessage = String.format("Passed configuration %s is not supported", paramKey); + LOG.error(anotherMessage); + } + } catch (XmlRpcException | Types.XenAPIException e) { + LOG.error("Exception caught while setting VM configuration"); + throw new CloudRuntimeException("Exception caught while setting VM configuration: " + cfg, e); + } + } else { + LOG.error("Unsupported extra configuration has been passed: " + paramKey); + throw new InvalidParameterValueException("Unsupported extra configuration parameter key has been passed: " + paramKey); + } + } + } + } + + private static boolean isValidOperation(Map recordMap, String actualParam) { + return recordMap.containsKey(actualParam); + } + + private static Map prepareKeyValuePair(String cfg) { + Map configKeyPair = new HashMap<>(); + int indexOfEqualSign = cfg.indexOf("="); + String key = cfg.substring(0, indexOfEqualSign).replace("-", "_"); + String value = cfg.substring(indexOfEqualSign + 1); + configKeyPair.put(key, value); + return configKeyPair; + } +} \ No newline at end of file diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 2843d6e456db..68f73a1784b8 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.vm; +import java.io.IOException; +import java.io.StringReader; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.util.ArrayList; @@ -34,11 +36,16 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.inject.Inject; import javax.naming.ConfigurationException; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -98,8 +105,11 @@ import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.w3c.dom.Document; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; -import com.cloud.hypervisor.kvm.dpdk.DpdkHelper; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; @@ -184,6 +194,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorCapabilitiesVO; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; +import com.cloud.hypervisor.kvm.dpdk.DpdkHelper; import com.cloud.network.IpAddressManager; import com.cloud.network.Network; import com.cloud.network.Network.IpAddresses; @@ -262,7 +273,6 @@ import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; -import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.SSHKeyPair; import com.cloud.user.SSHKeyPairVO; @@ -526,8 +536,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey AllowDeployVmIfGivenHostFails = new ConfigKey("Advanced", Boolean.class, "allow.deploy.vm.if.deploy.on.given.host.fails", "false", "allow vm to deploy on different host if vm fails to deploy on the given host ", true); - private static final ConfigKey EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, "enable.additional.vm.configuration", - "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account); + private static final ConfigKey EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, + "enable.additional.vm.configuration", "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account); + + private static final ConfigKey KvmAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.kvm", "", "Comma separated list of allowed additional configuration options.", true); + + private static final ConfigKey XenServerAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.xenserver", "", "Comma separated list of allowed additional configuration options", true); + + private static final ConfigKey VmwareAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.vmware", "", "Comma separated list of allowed additional configuration options.", true); + private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); @@ -2469,8 +2489,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx _vmDao.saveDetails(vmInstance); } if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(accountId)) { - AccountVO account = _accountDao.findById(accountId); - addExtraConfig(vmInstance, account, extraConfig); + addExtraConfig(vmInstance, extraConfig); } } return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, osTypeId, userData, isDynamicallyScalable, @@ -5025,7 +5044,8 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE Long callerId = caller.getId(); String extraConfig = cmd.getExtraConfig(); if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(callerId) ) { - addExtraConfig(vm, caller, extraConfig); + s_logger.info("Adding extra configuration to user vm: " + vm.getId()); + addExtraConfig(vm, extraConfig); } if (cmd.getCopyImageTags()) { @@ -5044,24 +5064,124 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE } /** - * Persist extra configurations as details for VMware VMs + * Persist extra configuration data in the user_vm_details table as key/value pair + * @param decodedUrl String consisting of the extra config data to appended onto the vmx file for VMware instances */ protected void persistExtraConfigVmware(String decodedUrl, UserVm vm) { - String[] configDataArr = decodedUrl.split("\\r?\\n"); - for (String config: configDataArr) { - String[] keyValue = config.split("="); - try { - userVmDetailsDao.addDetail(vm.getId(), keyValue[0], keyValue[1], true); - } catch (ArrayIndexOutOfBoundsException e) { - throw new CloudRuntimeException("Issue occurred during parsing of:" + config); + boolean isValidConfig = isValidKeyValuePair(decodedUrl); + if (isValidConfig) { + String[] extraConfigs = decodedUrl.split("\\r?\\n"); + for (String cfg : extraConfigs) { + // Validate cfg against unsupported operations set by admin here + String[] allowedKeyList = VmwareAdditionalConfigAllowList.value().split(","); + boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList); + String[] paramArray = cfg.split("="); + if (validXenOrVmwareConfiguration && paramArray.length == 2) { + try { + userVmDetailsDao.addDetail(vm.getId(), paramArray[0].trim(), paramArray[1].trim(), true); + } catch (ArrayIndexOutOfBoundsException e) { + throw new CloudRuntimeException("Issue occurred during parsing of:" + cfg); + } + + } else { + throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for VMware hypervisor hosts."); + } + } + } else { + throw new CloudRuntimeException("The passed extra config string " + decodedUrl + "contains an invalid key/value pair pattern"); + } + } + + /** + * Used to persist extra configuration settings in user_vm_details table for the XenServer hypervisor + * persists config as key/value pair e.g key = extraconfig-1 , value="PV-bootloader=pygrub" and so on to extraconfig-N where + * N denotes the number of extra configuration settings passed by user + * + * @param decodedUrl A string containing extra configuration settings as key/value pairs seprated by newline escape character + * e.x PV-bootloader=pygrub\nPV-args=console\nHV-Boot-policy="" + */ + protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) { + boolean isValidConfig = isValidKeyValuePair(decodedUrl); + if (isValidConfig) { + String[] extraConfigs = decodedUrl.split("\\r?\\n"); + int i = 1; + String extraConfigKey = ApiConstants.EXTRA_CONFIG + "-"; + for (String cfg : extraConfigs) { + // Validate cfg against unsupported operations set by admin here + String[] allowedKeyList = XenServerAdditionalConfigAllowList.value().split(","); + boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList); + if (validXenOrVmwareConfiguration) { + userVmDetailsDao.addDetail(vm.getId(), extraConfigKey + String.valueOf(i), cfg, true); + i++; + } else { + throw new CloudRuntimeException("Configuration " + cfg + " contains a blacklisted key by Root admin"); + } + } + } else { + String msg = String.format("The passed extra config string '%s' contains an invalid key/value pair pattern", decodedUrl); + throw new CloudRuntimeException(msg); + } + } + + /** + * Used to valid extraconfig keylvalue pair for Vmware and XenServer + * Example of tested valid config for VMware as taken from VM instance vmx file + *

+ * nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b + * migrate.hostLog=i-2-67-VM-5130f8ab.hlog + * ethernet0.address=02:00:5f:51:00:41 + *

+ *

+ * Examples of tested valid configs for XenServer + *

+ * is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0 + *

+ * + * Allow the following character set {', ", -, ., =, a-z, 0-9, empty space, \n} + * + * @param decodedUrl String conprising of extra config key/value pairs for XenServer and Vmware + * @return True if extraconfig is valid key/value pair + */ + protected boolean isValidKeyValuePair(String decodedUrl) { + // Valid pairs should look like "key-1=value1, param:key-2=value2, my.config.v0=False" + Pattern pattern = Pattern.compile("^(?:[\\w-\\s\\.:]*=[\\w-\\s\\.'\":]*(?:\\s+|$))+$"); + Matcher matcher = pattern.matcher(decodedUrl); + return matcher.matches(); + } + + /** + * Validates key/value pair strings passed as extra configuration for XenServer and Vmware + * @param cfg configuration key-value pair + * @param allowedKeyList list of allowed configuration keys for XenServer and VMware + * @return + */ + protected boolean isValidXenOrVmwareConfiguration(String cfg, String[] allowedKeyList) { + // This should be of minimum length 1 + // Value is ignored in case it is empty + String[] cfgKeyValuePair = cfg.split("="); + if (cfgKeyValuePair.length >= 1) { + for (String allowedKey : allowedKeyList) { + if (cfgKeyValuePair[0].equalsIgnoreCase(allowedKey.trim())) { + return true; + } } + } else { + String msg = String.format("An incorrect configuration %s has been passed", cfg); + throw new CloudRuntimeException(msg); } + return false; } /** - * Persist extra configurations as details for hypervisors except Vmware + * Persist extra configuration data on KVM + * persisted in the user_vm_details DB as extraconfig-1, and so on depending on the number of configurations + * For KVM, extra config is passed as XML + * @param decodedUrl string containing xml configuration to be persisted into user_vm_details table + * @param vm */ - protected void persistExtraConfigNonVmware(String decodedUrl, UserVm vm) { + protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) { + // validate config against blacklisted cfg commands + validateKvmExtraConfig(decodedUrl); String[] extraConfigs = decodedUrl.split("\n\n"); for (String cfg : extraConfigs) { int i = 1; @@ -5069,7 +5189,7 @@ protected void persistExtraConfigNonVmware(String decodedUrl, UserVm vm) { String extraConfigKey = ApiConstants.EXTRA_CONFIG; String extraConfigValue; if (cfgParts[0].matches("\\S+:$")) { - extraConfigKey += "-" + cfgParts[0].substring(0,cfgParts[0].length() - 1); + extraConfigKey += "-" + cfgParts[0].substring(0, cfgParts[0].length() - 1); extraConfigValue = cfg.replace(cfgParts[0] + "\n", ""); } else { extraConfigKey += "-" + String.valueOf(i); @@ -5080,16 +5200,65 @@ protected void persistExtraConfigNonVmware(String decodedUrl, UserVm vm) { } } - protected void addExtraConfig(UserVm vm, Account caller, String extraConfig) { + /** + * This method is called by the persistExtraConfigKvm + * Validates passed extra configuration data for KVM and validates against blacklist of unwanted commands + * controlled by Root admin + * @param decodedUrl string containing xml configuration to be validated + */ + protected void validateKvmExtraConfig(String decodedUrl) { + String[] allowedConfigOptionList = KvmAdditionalConfigAllowList.value().split(","); + String msg = "An invalid extra configuration option has been supplied: "; + // Skip allowed keys validation validation for DPDK + if (!decodedUrl.contains(":")) { + try { + DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + InputSource src = new InputSource(); + src.setCharacterStream(new StringReader(String.format("\n%s\n", decodedUrl))); + Document doc = builder.parse(src); + doc.getDocumentElement().normalize(); + for (String tag : allowedConfigOptionList) { + NodeList nodeList = doc.getElementsByTagName(tag.trim()); + // Node list should not be empty to show that allowed command is contained in passed XML + if (nodeList.getLength() == 0) { + throw new CloudRuntimeException(msg + tag); + } + } + } catch (ParserConfigurationException | IOException | SAXException e) { + throw new CloudRuntimeException("Failed to parse additional XML configuration: " + e.getMessage()); + } + } + } + + /** + * Adds extra config data to guest VM instances + * @param extraConfig Extra Configuration settings to be added in UserVm instances for KVM, XenServer and VMware + */ + protected void addExtraConfig(UserVm vm, String extraConfig) { String decodedUrl = decodeExtraConfig(extraConfig); HypervisorType hypervisorType = vm.getHypervisorType(); - if (hypervisorType == HypervisorType.VMware) { - persistExtraConfigVmware(decodedUrl, vm); - } else { - persistExtraConfigNonVmware(decodedUrl, vm); + + switch (hypervisorType) { + case XenServer: + persistExtraConfigXenServer(decodedUrl, vm); + break; + case KVM: + persistExtraConfigKvm(decodedUrl, vm); + break; + case VMware: + persistExtraConfigVmware(decodedUrl, vm); + break; + default: + String msg = String.format("This hypervisor %s is not supported for use with this feature", hypervisorType.toString()); + throw new CloudRuntimeException(msg); } } + /** + * Decodes an URL encoded string passed as extra configuration for guest VMs + * @param encodeString URL encoded string + * @return String result of decoded URL + */ protected String decodeExtraConfig(String encodeString) { String decodedUrl; try { @@ -6671,7 +6840,8 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax, - VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig}; + VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, KvmAdditionalConfigAllowList, + XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList}; } @Override diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py new file mode 100644 index 000000000000..5fb4d5657235 --- /dev/null +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -0,0 +1,453 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" BVT tests for Virtual Machine additional configuration +""" +# Import System modules +import urllib +import xml.etree.ElementTree as ET + +from lxml import etree +from marvin.cloudstackAPI import (updateVirtualMachine, + deployVirtualMachine, + destroyVirtualMachine, + stopVirtualMachine, + startVirtualMachine, + updateConfiguration) +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + ServiceOffering, + ) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + list_hosts) +from marvin.lib.utils import * +from nose.plugins.attrib import attr + + +class TestAddConfigtoDeployVM(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestAddConfigtoDeployVM, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.hypervisor = testClient.getHypervisorInfo() + cls.services['mode'] = cls.zone.networktype + cls.hostConfig = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][ + 0].__dict__ + + # Set Zones and disk offerings + cls.services["small"]["zoneid"] = cls.zone.id + + cls.services["iso1"]["zoneid"] = cls.zone.id + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + + # Create an account, network, and IP addresses + cls.account = Account.create( + cls.apiclient, + cls.services["account"], + domainid=cls.domain.id + ) + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["small"] + ) + + cls.cleanup = [ + cls.account, + cls.service_offering + ] + + @classmethod + def tearDownClass(cls): + try: + cls.apiclient = super(TestAddConfigtoDeployVM, cls).getClsTestClient().getApiClient() + # Clean up, terminate the created templates + cleanup_resources(cls.apiclient, cls.cleanup) + + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + + """ + Set EnableAdditionalData to true + """ + updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() + updateConfigurationCmd.name = "enable.additional.vm.configuration" + updateConfigurationCmd.value = "true" + updateConfigurationCmd.scopename = "account" + updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) + self.debug("updated the parameter %s with value %s" % ( + updateConfigurationResponse.name, updateConfigurationResponse.value)) + + # Ste Global Config value + def add_global_config(self, name, value): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + + cmd = updateConfiguration.updateConfigurationCmd() + cmd.name = name + cmd.value = value + return self.apiclient.updateConfigurationCmd(cmd) + + + # Compare XML Element objects + def elements_equal(self, e1, e2): + if e1.tag != e2.tag: + return False + if e1.attrib != e2.attrib: + return False + if len(e1) != len(e2): + return False + return all(self.elements_equal(c1, c2) for c1, c2 in zip(e1, e2)) + + def destroy_vm(self, vm_id): + cmd = destroyVirtualMachine.destroyVirtualMachineCmd() + cmd.expunge = True + cmd.id = vm_id + return self.apiclient.destroyVirtualMachine(cmd) + + def deploy_vm(self, hypervisor, extra_config=None): + cmd = deployVirtualMachine.deployVirtualMachineCmd() + if extra_config is not None: + cmd.extraconfig = extra_config + + template = get_template( + self.apiclient, + self.zone.id, + hypervisor=hypervisor + ) + cmd.zoneid = self.zone.id + cmd.templateid = template.id + cmd.serviceofferingid = self.service_offering.id + return self.apiclient.deployVirtualMachine(cmd) + + def update_vm(self, id, extra_config): + cmd = updateVirtualMachine.updateVirtualMachineCmd() + cmd.id = id + cmd.extraconfig = extra_config + return self.apiclient.updateVirtualMachine(cmd) + + def stop_vm(self, id): + cmd = stopVirtualMachine.stopVirtualMachineCmd() + cmd.id = id + return self.apiclient.stopVirtualMachine(cmd) + + def start_vm(self, id): + cmd = startVirtualMachine.startVirtualMachineCmd() + cmd.id = id + return self.apiclient.startVirtualMachine(cmd) + + # Parse extraconfig for config with that returned by xe vm-param-get ... + def get_xen_param_values(self, config): + equal_sign_index = config.index("=") + cmd_option = config[:equal_sign_index] + cmd_value = config[equal_sign_index + 1:] + return cmd_option, cmd_value + + # Format vm config such that it equals the one from vmx file + def prepare_vmware_config(self, config): + equal_sign_index = config.index("=") + cmd_option = config[:equal_sign_index] + cmd_value = config[equal_sign_index + 1:] + return cmd_option + ' = ' '"{}"'.format(cmd_value) + + # Get vm uuid from xenserver host + def get_vm_uuid(self, instance_name, ssh_client): + cmd = 'xe vm-list name-label={} params=uuid '.format(instance_name) + result = ssh_client.execute(cmd) + uuid_str = result[0] + i = uuid_str.index(":") + return uuid_str[i + 1:].strip() + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_deploy_vm__with_extraconfig_kvm(self): + ''' + Test that extra config is added on KVM hosts + ''' + + if self.hypervisor.lower() != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + name = 'allow.additional.vm.configuration.list.kvm' + value = 'memoryBacking, hugepages' + + add_config_response = self.add_global_config(name, value) + + self.assertTrue( + name, + add_config_response.name, + "Failed to set global setting " + name + "on KVM" + ) + + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm('kvm', extraconfig) + + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor='kvm') + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(response.id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_update_vm__with_extraconfig_kvm(self): + ''' + Test that extra config is added on KVM hosts + ''' + if self.hypervisor.lower() != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + name = 'allow.additional.vm.configuration.list.kvm' + value = 'memoryBacking, hugepages' + + add_config_response = self.add_global_config(name, value) + + self.assertTrue( + name, + add_config_response.name, + "Failed to set global setting " + name + "on KVM" + ) + + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm('kvm') + vm_id = response.id + + ''' + For updateVirtualMachineCmd, the VM must be stopped and restarted for changes to take effect + ''' + self.stop_vm(vm_id) + self.update_vm(vm_id, extraconfig) + start_resp = self.start_vm(vm_id) + + host_id = start_resp.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor='kvm') + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(vm_id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_deploy_vm__with_extraconfig_vmware(self): + ''' + Test that extra config is added on VMware hosts + ''' + if self.hypervisor.lower() != 'vmware': + raise self.skipTest("Skipping test case for non-vmware hypervisor") + + name = 'allow.additional.vm.configuration.list.vmware' + value = 'hypervisor.cpuid.v0' + + add_config_response = self.add_global_config(name, value) + + self.assertTrue( + name, + add_config_response.name, + "Failed to set global setting " + name + "on VMware" + ) + + ''' + The following extra configuration is used to set Hyper-V instance to run on ESXi host + + ''' + extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + try: + response = self.deploy_vm('vmware', extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + for config in config_arr: + vmx_config = self.prepare_vmware_config(config) + vmx_file_name = "\"$(esxcli vm process list | grep %s | tail -1 | awk '{print $3}')\"" % instance_name + # parse vm instance vmx file to see if extraconfig has been added + grep_config = "cat %s | grep -w '%s'" % (vmx_file_name, vmx_config) + result = ssh_client.execute(grep_config) + # Match exact configuration from vmx file, return empty result array if configuration is not found + self.assertNotEquals( + 0, + len(result), + 'Extra configuration not found in instance vmx file' + ) + finally: + self.destroy_vm(response.id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_deploy_vm__with_extraconfig_xenserver(self): + if self.hypervisor.lower() != 'xenserver': + raise self.skipTest("Skipping test case for non-xenserver hypervisor") + """ + Following commands are used to convert a VM from HVM to PV and set using vm-param-set + HVM-boot-policy= + PV-bootloader=pygrub + PV-args=hvc0 + """ + + name = 'allow.additional.vm.configuration.list.xenserver' + value = 'HVM-boot-policy, PV-bootloader, PV-args' + + add_config_response = self.add_global_config(name, value) + + self.assertTrue( + name, + add_config_response.name, + "Failed to set global setting " + name + "on Xenserver" + ) + extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' + try: + response = self.deploy_vm('xenserver', extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + # Get vm instance uuid + instance_uuid = self.get_vm_uuid(response.instancename, ssh_client) + for config in config_arr: + config_tuple = self.get_xen_param_values(config) + # Log on to XenServer host and check the vm-param-get + vm_config_check = 'xe vm-param-get param-name={} uuid={}'.format(config_tuple[0], instance_uuid) + result = ssh_client.execute(vm_config_check) + param_value = config_tuple[1].strip() + # Check if each configuration command has set the configuration as sent with extraconfig + self.assertEquals( + param_value, + result[0], + 'Extra configuration not found in VM param list' + ) + finally: + self.destroy_vm(response.id) From be39548ea41de98352f2784731124885ec81f4cc Mon Sep 17 00:00:00 2001 From: Dingane Hlaluku Date: Thu, 7 Mar 2019 15:23:06 +0200 Subject: [PATCH 09/10] * Fix marvin test failure * Add new marvin negative tests cases * Remove hard-coded hypervisor types in marvin tests --- .../cloud/vm/VirtualMachineManagerImpl.java | 4 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 5 +- .../com/cloud/vm/UserVmManagerImplTest.java | 19 +- .../smoke/test_deploy_vm_extra_config_data.py | 495 ++++++++++-------- 4 files changed, 307 insertions(+), 216 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index e9a9f074f906..3504f6686528 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -39,8 +39,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.agent.api.PrepareForMigrationAnswer; -import com.cloud.agent.api.to.DpdkTO; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd; import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin; @@ -95,6 +93,7 @@ import com.cloud.agent.api.PingRoutingCommand; import com.cloud.agent.api.PlugNicAnswer; import com.cloud.agent.api.PlugNicCommand; +import com.cloud.agent.api.PrepareForMigrationAnswer; import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.RebootAnswer; import com.cloud.agent.api.RebootCommand; @@ -114,6 +113,7 @@ import com.cloud.agent.api.UnregisterVMCommand; import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.DpdkTO; import com.cloud.agent.api.to.GPUDeviceTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.VirtualMachineTO; diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 68f73a1784b8..07661c2a14e3 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -5114,7 +5114,7 @@ protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) { userVmDetailsDao.addDetail(vm.getId(), extraConfigKey + String.valueOf(i), cfg, true); i++; } else { - throw new CloudRuntimeException("Configuration " + cfg + " contains a blacklisted key by Root admin"); + throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for XenServer hypervisor hosts."); } } } else { @@ -5208,7 +5208,6 @@ protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) { */ protected void validateKvmExtraConfig(String decodedUrl) { String[] allowedConfigOptionList = KvmAdditionalConfigAllowList.value().split(","); - String msg = "An invalid extra configuration option has been supplied: "; // Skip allowed keys validation validation for DPDK if (!decodedUrl.contains(":")) { try { @@ -5221,7 +5220,7 @@ protected void validateKvmExtraConfig(String decodedUrl) { NodeList nodeList = doc.getElementsByTagName(tag.trim()); // Node list should not be empty to show that allowed command is contained in passed XML if (nodeList.getLength() == 0) { - throw new CloudRuntimeException(msg + tag); + throw new CloudRuntimeException(String.format("Extra config %s is not on the list of allowed keys for KVM hypervisor hosts", tag)); } } } catch (ParserConfigurationException | IOException | SAXException e) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 965377b2c7bc..f5bfa2ba950f 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -16,6 +16,10 @@ // under the License. package com.cloud.vm; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.HashMap; @@ -23,7 +27,6 @@ import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; import org.apache.cloudstack.context.CallContext; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -291,6 +294,18 @@ private void configureValidateOrReplaceMacAddressTest(int times, String macAddre String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, 1l); Mockito.verify(networkModel, Mockito.times(times)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); - Assert.assertEquals(expectedMacAddress, returnedMacAddress); + assertEquals(expectedMacAddress, returnedMacAddress); + } + + @Test + public void testValidatekeyValuePair() throws Exception { + assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true HVM-boot-policy= PV-bootloader=pygrub PV-args=hvc0")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b")); + assertFalse(userVmManagerImpl.isValidKeyValuePair("key")); + //key-1=value1, param:key-2=value2, my.config.v0=False" + assertTrue(userVmManagerImpl.isValidKeyValuePair("key-1=value1")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("param:key-2=value2")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("my.config.v0=False")); } } diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py index 5fb4d5657235..d59748d76ee5 100644 --- a/test/integration/smoke/test_deploy_vm_extra_config_data.py +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -114,8 +114,7 @@ def add_global_config(self, name, value): cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value - return self.apiclient.updateConfigurationCmd(cmd) - + return self.apiclient.updateConfiguration(cmd) # Compare XML Element objects def elements_equal(self, e1, e2): @@ -187,85 +186,114 @@ def get_vm_uuid(self, instance_name, ssh_client): return uuid_str[i + 1:].strip() @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") - def test_deploy_vm__with_extraconfig_kvm(self): + def test_01_deploy_vm__with_extraconfig_throws_exception_kvm(self): + ''' + Test that extra config is not added when element tag is not added on the allowed list global config on KVM hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + try: + # Clear KVM allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.kvm' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check kvm global configuration") + except Exception as e: + logging.debug(e) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_02_deploy_vm__with_extraconfig_kvm(self): ''' Test that extra config is added on KVM hosts ''' - if self.hypervisor.lower() != 'kvm': + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': raise self.skipTest("Skipping test case for non-kvm hypervisor") name = 'allow.additional.vm.configuration.list.kvm' - value = 'memoryBacking, hugepages' + value = 'memoryBacking' add_config_response = self.add_global_config(name, value) - self.assertTrue( - name, - add_config_response.name, - "Failed to set global setting " + name + "on KVM" - ) - - try: - ''' - The following extraconfig is required for enabling hugepages on kvm - - - - url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' - ''' - extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" - - response = self.deploy_vm('kvm', extraconfig) - - host_id = response.hostid - host = list_hosts( - self.apiclient, - id=host_id, - hypervisor='kvm') - - instance_name = response.instancename - host_ipaddress = host[0].ipaddress - - ssh_client = SshClient(host_ipaddress, port=22, - user=self.hostConfig['username'], - passwd=self.hostConfig['password']) - virsh_cmd = 'virsh dumpxml %s' % instance_name - xml_res = ssh_client.execute(virsh_cmd) - xml_as_str = ''.join(xml_res) - - extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' - - # Root XML Elements - parser = etree.XMLParser(remove_blank_text=True) - domain_xml_root = ET.fromstring(xml_as_str, parser=parser) - decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) - for child in decoded_xml_root: - find_element_in_domain_xml = domain_xml_root.find(child.tag) - - # Fail if extra config is not found in domain xml - self.assertNotEquals( - 0, - len(find_element_in_domain_xml), - 'Element tag from extra config not added to VM' - ) - - # Compare found XML node with extra config node - is_a_match = self.elements_equal(child, find_element_in_domain_xml) - self.assertEquals( - True, - is_a_match, - 'The element from tags from extra config do not match with those found in domain xml' - ) - finally: - self.destroy_vm(response.id) + if add_config_response.name: + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm(hypervisor, extraconfig) + + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor=hypervisor) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "") @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") - def test_update_vm__with_extraconfig_kvm(self): + def test_03_update_vm__with_extraconfig_kvm(self): ''' Test that extra config is added on KVM hosts ''' - if self.hypervisor.lower() != 'kvm': + + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': raise self.skipTest("Skipping test case for non-kvm hypervisor") name = 'allow.additional.vm.configuration.list.kvm' @@ -273,80 +301,104 @@ def test_update_vm__with_extraconfig_kvm(self): add_config_response = self.add_global_config(name, value) - self.assertTrue( - name, - add_config_response.name, - "Failed to set global setting " + name + "on KVM" - ) + if add_config_response.name: + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm(hypervisor) + vm_id = response.id + + ''' + For updateVirtualMachineCmd, the VM must be stopped and restarted for changes to take effect + ''' + self.stop_vm(vm_id) + self.update_vm(vm_id, extraconfig) + start_resp = self.start_vm(vm_id) + + host_id = start_resp.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor=hypervisor) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(vm_id) + self.add_global_config(name, "") - try: - ''' - The following extraconfig is required for enabling hugepages on kvm - - - - url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' - ''' - extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_05_deploy_vm__with_extraconfig_throws_exception_vmware(self): + ''' + Test that extra config is not added when configuration key is not added on the allowed list global config for VMWARE hosts + ''' - response = self.deploy_vm('kvm') - vm_id = response.id + hypervisor = self.hypervisor.lower() + if hypervisor != 'vmware': + raise self.skipTest("Skipping test case for non-vmware hypervisor") - ''' - For updateVirtualMachineCmd, the VM must be stopped and restarted for changes to take effect - ''' - self.stop_vm(vm_id) - self.update_vm(vm_id, extraconfig) - start_resp = self.start_vm(vm_id) - - host_id = start_resp.hostid - host = list_hosts( - self.apiclient, - id=host_id, - hypervisor='kvm') - - instance_name = response.instancename - host_ipaddress = host[0].ipaddress - - ssh_client = SshClient(host_ipaddress, port=22, - user=self.hostConfig['username'], - passwd=self.hostConfig['password']) - virsh_cmd = 'virsh dumpxml %s' % instance_name - xml_res = ssh_client.execute(virsh_cmd) - xml_as_str = ''.join(xml_res) - - extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' - - # Root XML Elements - parser = etree.XMLParser(remove_blank_text=True) - domain_xml_root = ET.fromstring(xml_as_str, parser=parser) - decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) - for child in decoded_xml_root: - find_element_in_domain_xml = domain_xml_root.find(child.tag) - - # Fail if extra config is not found in domain xml - self.assertNotEquals( - 0, - len(find_element_in_domain_xml), - 'Element tag from extra config not added to VM' - ) - - # Compare found XML node with extra config node - is_a_match = self.elements_equal(child, find_element_in_domain_xml) - self.assertEquals( - True, - is_a_match, - 'The element from tags from extra config do not match with those found in domain xml' - ) - finally: - self.destroy_vm(vm_id) + ''' + The following extra configuration is used to set Hyper-V instance to run on ESXi host + hypervisor.cpuid.v0 = FALSE + ''' + extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + + try: + # Clear VMWARE allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.vmware' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check VMWARE global configuration") + except Exception as e: + logging.debug(e) @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") - def test_deploy_vm__with_extraconfig_vmware(self): + def test_06_deploy_vm__with_extraconfig_vmware(self): ''' Test that extra config is added on VMware hosts ''' - if self.hypervisor.lower() != 'vmware': + hypervisor = self.hypervisor.lower() + if hypervisor != 'vmware': raise self.skipTest("Skipping test case for non-vmware hypervisor") name = 'allow.additional.vm.configuration.list.vmware' @@ -354,52 +406,80 @@ def test_deploy_vm__with_extraconfig_vmware(self): add_config_response = self.add_global_config(name, value) - self.assertTrue( - name, - add_config_response.name, - "Failed to set global setting " + name + "on VMware" - ) + if add_config_response.name: + + ''' + The following extra configuration is used to set Hyper-V instance to run on ESXi host + hypervisor.cpuid.v0 = FALSE + ''' + extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + try: + response = self.deploy_vm(hypervisor, extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + for config in config_arr: + vmx_config = self.prepare_vmware_config(config) + vmx_file_name = "\"$(esxcli vm process list | grep %s | tail -1 | awk '{print $3}')\"" % instance_name + # parse vm instance vmx file to see if extraconfig has been added + grep_config = "cat %s | grep -w '%s'" % (vmx_file_name, vmx_config) + result = ssh_client.execute(grep_config) + # Match exact configuration from vmx file, return empty result array if configuration is not found + self.assertNotEquals( + 0, + len(result), + 'Extra configuration not found in instance vmx file' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "") + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_07_deploy_vm__with_extraconfig_throws_exception_xenserver(self): + ''' + Test that extra config is not added when configuration key is not added on the allowed list global config for XenServer hosts ''' - The following extra configuration is used to set Hyper-V instance to run on ESXi host + + hypervisor = self.hypervisor.lower() + if hypervisor != 'xenserver': + raise self.skipTest("Skipping test case for non-xenserver hypervisor") ''' - extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + Following commands are used to convert a VM from HVM to PV and set using vm-param-set + HVM-boot-policy= + PV-bootloader=pygrub + PV-args=hvc0 + ''' + + extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' + try: - response = self.deploy_vm('vmware', extraconfig) - host_id = response.hostid - host = list_hosts( - self.apiclient, - id=host_id) - - instance_name = response.instancename - host_ipaddress = host[0].ipaddress - - ssh_client = SshClient(host_ipaddress, port=22, - user=self.hostConfig['username'], - passwd=self.hostConfig['password']) - - extraconfig_decoded = urllib.unquote(extraconfig) - config_arr = extraconfig_decoded.splitlines() - - for config in config_arr: - vmx_config = self.prepare_vmware_config(config) - vmx_file_name = "\"$(esxcli vm process list | grep %s | tail -1 | awk '{print $3}')\"" % instance_name - # parse vm instance vmx file to see if extraconfig has been added - grep_config = "cat %s | grep -w '%s'" % (vmx_file_name, vmx_config) - result = ssh_client.execute(grep_config) - # Match exact configuration from vmx file, return empty result array if configuration is not found - self.assertNotEquals( - 0, - len(result), - 'Extra configuration not found in instance vmx file' - ) - finally: - self.destroy_vm(response.id) + # Clear VMWARE allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.xenserver' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check XenServer global configuration") + except Exception as e: + logging.debug(e) @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") - def test_deploy_vm__with_extraconfig_xenserver(self): - if self.hypervisor.lower() != 'xenserver': + def test_08_deploy_vm__with_extraconfig_xenserver(self): + hypervisor = self.hypervisor.lower() + if hypervisor != 'xenserver': raise self.skipTest("Skipping test case for non-xenserver hypervisor") """ Following commands are used to convert a VM from HVM to PV and set using vm-param-set @@ -413,41 +493,38 @@ def test_deploy_vm__with_extraconfig_xenserver(self): add_config_response = self.add_global_config(name, value) - self.assertTrue( - name, - add_config_response.name, - "Failed to set global setting " + name + "on Xenserver" - ) - extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' - try: - response = self.deploy_vm('xenserver', extraconfig) - host_id = response.hostid - host = list_hosts( - self.apiclient, - id=host_id) - - host_ipaddress = host[0].ipaddress - - ssh_client = SshClient(host_ipaddress, port=22, - user=self.hostConfig['username'], - passwd=self.hostConfig['password']) - - extraconfig_decoded = urllib.unquote(extraconfig) - config_arr = extraconfig_decoded.splitlines() - - # Get vm instance uuid - instance_uuid = self.get_vm_uuid(response.instancename, ssh_client) - for config in config_arr: - config_tuple = self.get_xen_param_values(config) - # Log on to XenServer host and check the vm-param-get - vm_config_check = 'xe vm-param-get param-name={} uuid={}'.format(config_tuple[0], instance_uuid) - result = ssh_client.execute(vm_config_check) - param_value = config_tuple[1].strip() - # Check if each configuration command has set the configuration as sent with extraconfig - self.assertEquals( - param_value, - result[0], - 'Extra configuration not found in VM param list' - ) - finally: - self.destroy_vm(response.id) + if add_config_response.name: + extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' + try: + response = self.deploy_vm(hypervisor, extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + # Get vm instance uuid + instance_uuid = self.get_vm_uuid(response.instancename, ssh_client) + for config in config_arr: + config_tuple = self.get_xen_param_values(config) + # Log on to XenServer host and check the vm-param-get + vm_config_check = 'xe vm-param-get param-name={} uuid={}'.format(config_tuple[0], instance_uuid) + result = ssh_client.execute(vm_config_check) + param_value = config_tuple[1].strip() + # Check if each configuration command has set the configuration as sent with extraconfig + self.assertEquals( + param_value, + result[0], + 'Extra configuration not found in VM param list' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "") From d0fcd976a9a3dab50444ef69b5026838ae64dd51 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Tue, 23 Jul 2019 15:18:04 +0530 Subject: [PATCH 10/10] Fix build error after rebase and add hugepagesless --- .../src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java | 1 + test/integration/smoke/test_deploy_vm_extra_config_data.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 3504f6686528..2d5061be6f59 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -40,6 +40,7 @@ import javax.naming.ConfigurationException; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd; import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py index d59748d76ee5..4ee900c72e72 100644 --- a/test/integration/smoke/test_deploy_vm_extra_config_data.py +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -226,7 +226,7 @@ def test_02_deploy_vm__with_extraconfig_kvm(self): raise self.skipTest("Skipping test case for non-kvm hypervisor") name = 'allow.additional.vm.configuration.list.kvm' - value = 'memoryBacking' + value = 'memoryBacking, hugepagesless' add_config_response = self.add_global_config(name, value)