weizhouapache commented on code in PR #6202: URL: https://github.com/apache/cloudstack/pull/6202#discussion_r876923247
########## api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java: ########## @@ -0,0 +1,122 @@ +// 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.user.userdata; + +import com.cloud.user.Account; +import com.cloud.user.UserData; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.UserDataResponse; +import org.apache.log4j.Logger; + +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.ProjectResponse; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.context.CallContext; + + +@APICommand(name = "deleteUserData", description = "Deletes a keypair by name", responseObject = SuccessResponse.class, entityType = {UserData.class}, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.18") +public class DeleteUserDataCmd extends BaseCmd { + + public static final Logger s_logger = Logger.getLogger(DeleteUserDataCmd.class.getName()); + private static final String s_name = "deleteuserdataresponse"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = UserDataResponse.class, description = "the ID of the Userdata") Review Comment: `required=true` for `id` ? ########## api/src/main/java/org/apache/cloudstack/api/command/user/userdata/LinkUserDataToTemplateCmd.java: ########## @@ -0,0 +1,118 @@ +// 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.user.userdata; + +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.user.Account; +import com.cloud.user.UserData; +import com.cloud.utils.exception.CloudRuntimeException; +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.ResponseObject; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.AdminCmd; +import org.apache.cloudstack.api.response.TemplateResponse; +import org.apache.cloudstack.api.response.UserDataResponse; +import org.apache.log4j.Logger; + +@APICommand(name = "linkUserDataToTemplate", description = "Links a userdata to a template.", responseObject = TemplateResponse.class, responseView = ResponseObject.ResponseView.Restricted, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) Review Comment: since = "4.18" ? ########## api/src/main/java/com/cloud/user/UserData.java: ########## @@ -0,0 +1,30 @@ +// 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.user; + +import org.apache.cloudstack.acl.ControlledEntity; +import org.apache.cloudstack.api.Identity; +import org.apache.cloudstack.api.InternalIdentity; + +public interface UserData extends ControlledEntity, InternalIdentity, Identity { + + public enum UserDataOverridePolicy { allowoverride, append, denyoverride } Review Comment: does this meet the java code convention ? ########## engine/schema/src/main/java/com/cloud/vm/UserVmVO.java: ########## @@ -42,6 +42,12 @@ public class UserVmVO extends VMInstanceVO implements UserVm { @Basic(fetch = FetchType.LAZY) private String userData; + @Column(name = "user_data_id", nullable = true, length = 17) Review Comment: length might be unnecessary for `user_data_id` ########## server/src/main/java/com/cloud/server/ManagementServerImpl.java: ########## @@ -4417,6 +4625,20 @@ private SSHKeyPair createAndSaveSSHKeyPair(final String name, final String finge return newPair; } + private UserData createAndSaveUserData(final String name, final String userdata, final String params, final Account owner) { + final UserDataVO userDataVO = new UserDataVO(); + + userDataVO.setAccountId(owner.getAccountId()); + userDataVO.setDomainId(owner.getDomainId()); + userDataVO.setName(name); + userDataVO.setUserData(userdata); + userDataVO.setParams(params); Review Comment: is validation on `params` required ? e.g. format, and reserved keys ########## api/src/main/java/org/apache/cloudstack/api/command/user/userdata/LinkUserDataToTemplateCmd.java: ########## @@ -0,0 +1,118 @@ +// 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.user.userdata; + +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.user.Account; +import com.cloud.user.UserData; +import com.cloud.utils.exception.CloudRuntimeException; +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.ResponseObject; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.AdminCmd; +import org.apache.cloudstack.api.response.TemplateResponse; +import org.apache.cloudstack.api.response.UserDataResponse; +import org.apache.log4j.Logger; + +@APICommand(name = "linkUserDataToTemplate", description = "Links a userdata to a template.", responseObject = TemplateResponse.class, responseView = ResponseObject.ResponseView.Restricted, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class LinkUserDataToTemplateCmd extends BaseCmd implements AdminCmd { + public static final Logger s_logger = Logger.getLogger(LinkUserDataToTemplateCmd.class.getName()); + + private static final String s_name = "linkuserdatatotemplateresponse"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.TEMPLATE_ID, + type = CommandType.UUID, + entityType = TemplateResponse.class, + required = true, + description = "the ID of the template for the virtual machine") + private Long templateId; + + @Parameter(name = ApiConstants.USER_DATA_ID, + type = CommandType.UUID, + entityType = UserDataResponse.class, + required = true, + description = "the ID of the userdata that has to be linked to template") + private Long userdataId; + + @Parameter(name = ApiConstants.USER_DATA_POLICY, + type = CommandType.STRING, + description = "an optional override policy of the userdata. Possible values are - allowoverride, append, denyoverride. Default policy is allowoverride") + private String userdataPolicy; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getTemplateId() { + return templateId; + } + + public Long getUserdataId() { + return userdataId; + } + + public UserData.UserDataOverridePolicy getUserdataPolicy() { + if (userdataPolicy == null) { + return UserData.UserDataOverridePolicy.allowoverride; + } + return UserData.UserDataOverridePolicy.valueOf(userdataPolicy); Review Comment: userdataPolicy.toLowerCase ? ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: + if (userDataId != null) { + if (userData != null) { Review Comment: use `StringUtils.isNotEmpty`? ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: + if (userDataId != null) { + if (userData != null) { + s_logger.info("Both userdata and userdata ID are provided, precedence goes to userdata ID"); + } + UserData apiUserDataVO = _userDataDao.findById(userDataId); + return apiUserDataVO.getUserData(); + } else if (userData != null) { + return userData; + } else { + UserData templateUserDataVO = _userDataDao.findById(template.getUserDataId()); + if (templateUserDataVO == null) { + String msg = String.format("UserData linked to the template %s is not found", template.getName()); + throw new CloudRuntimeException(msg); + } + return templateUserDataVO.getUserData(); + } + case append: Review Comment: better to use `UserDataOverridePolicy.append" ########## ui/src/views/compute/DeployVM.vue: ########## @@ -562,10 +562,97 @@ @change="val => { dynamicscalingenabled = val }"/> </a-form-item> </a-form-item> - <a-form-item :label="$t('label.userdata')" name="userdata" ref="userdata"> - <a-textarea - v-model:value="form.userdata"> - </a-textarea> + <a-form-item :label="$t('label.userdata')"> Review Comment: this works well. it looks the form to update vm, register Iso, upload iso from local, update iso are not changed. ########## api/src/main/java/org/apache/cloudstack/api/response/TemplateResponse.java: ########## @@ -227,6 +227,18 @@ public class TemplateResponse extends BaseResponseWithTagInformation implements @Param(description = "Base64 string representation of the resource icon", since = "4.16.0.0") ResourceIconResponse icon; + @SerializedName(ApiConstants.USER_DATA_ID) @Param(description="the id of userdata linked to this template", since = "4.18.0.0") Review Comment: it would be good to use the same value for `since` some api/params use `4.18` and some use `4.18.0.0` ########## engine/schema/src/main/java/com/cloud/user/UserDataVO.java: ########## @@ -0,0 +1,117 @@ +// 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.user; + +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 = "user_data") +public class UserDataVO implements UserData { + + public UserDataVO() { + uuid = UUID.randomUUID().toString(); + } + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id = null; + + @Column(name = "uuid") + private String uuid; + + @Column(name = "account_id") + private long accountId; + + @Column(name = "domain_id") + private long domainId; + + @Column(name = "name") + private String name; + + @Column(name = "user_data") Review Comment: the default `length` for Column is 255 it would be good to set to a large value for example, in UserVmVO.java, it has ``` @Column(name = "user_data", updatable = true, nullable = true, length = 1048576) @Basic(fetch = FetchType.LAZY) private String userData; ``` ########## engine/schema/src/main/resources/META-INF/db/schema-41610to41700.sql: ########## @@ -933,6 +933,331 @@ CREATE VIEW `cloud`.`event_view` AS `cloud`.`projects` ON projects.project_account_id = event.account_id LEFT JOIN `cloud`.`event` eve ON event.start_id = eve.id; +-- PR#5984 Update name for global configuration vm.stats.increment.metrics +Update configuration set name='vm.stats.increment.metrics' where name='vm.stats.increment.metrics.in.memory'; +CREATE TABLE `cloud`.`user_data` ( Review Comment: database changes should be put in 41700to41800.sql considering the upgrade path does not exist, we can temporarily put into 41610to41700.sql ########## engine/schema/src/main/resources/META-INF/db/schema-41610to41700.sql: ########## @@ -933,6 +933,331 @@ CREATE VIEW `cloud`.`event_view` AS `cloud`.`projects` ON projects.project_account_id = event.account_id LEFT JOIN `cloud`.`event` eve ON event.start_id = eve.id; +-- PR#5984 Update name for global configuration vm.stats.increment.metrics Review Comment: line 936 and 937 seem to be mistake in git merge. ########## engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java: ########## @@ -422,6 +422,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu InactiveUnremovedTmpltSearch.and("removed", InactiveUnremovedTmpltSearch.entity().getRemoved(), SearchCriteria.Op.NULL); InactiveUnremovedTmpltSearch.done(); + UserDataSearch = createSearchBuilder(); Review Comment: this might not work well when we remove a template from a zone, it will not set `removed` field in `vm_template` table. therefore this search result will contain the templates which have been removed from the zone. ########## api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java: ########## @@ -22,6 +22,20 @@ import java.util.Map; import java.util.Set; +import com.cloud.server.ResourceIcon; +import com.cloud.user.UserData; +import com.cloud.utils.Pair; +import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse; +import org.apache.cloudstack.api.response.ResourceIconResponse; +import org.apache.cloudstack.api.response.DirectDownloadCertificateHostStatusResponse; +import org.apache.cloudstack.api.response.RouterHealthCheckResultResponse; +import com.cloud.resource.RollingMaintenanceManager; +import org.apache.cloudstack.api.response.RollingMaintenanceResponse; +import org.apache.cloudstack.direct.download.DirectDownloadCertificate; +import org.apache.cloudstack.direct.download.DirectDownloadCertificateHostMap; +import org.apache.cloudstack.direct.download.DirectDownloadManager; +import org.apache.cloudstack.api.response.UserDataResponse; Review Comment: can these imports be ordered ? ########## engine/schema/src/main/java/com/cloud/vm/UserVmVO.java: ########## @@ -42,6 +42,12 @@ public class UserVmVO extends VMInstanceVO implements UserVm { @Basic(fetch = FetchType.LAZY) private String userData; + @Column(name = "user_data_id", nullable = true, length = 17) + private Long userDataId = null; + + @Column(name = "user_data_details", updatable = true) + private String userDataDetails; Review Comment: set `length` ? ########## server/src/main/java/com/cloud/network/element/CloudZonesNetworkElement.java: ########## @@ -158,6 +158,19 @@ private VmDataCommand generateVmDataCommand(String vmPrivateIpAddress, String us cmd.addVmData("metadata", "local-hostname", vmName); cmd.addVmData("metadata", "public-ipv4", guestIpAddress); cmd.addVmData("metadata", "public-hostname", guestIpAddress); + + if(userDataDetails != null && !userDataDetails.isEmpty()) { + userDataDetails = userDataDetails.substring(1, userDataDetails.length()-1); Review Comment: @harikrishna-patnala can you explain why `substring` is used here ? ########## server/src/main/java/com/cloud/server/ManagementServerImpl.java: ########## @@ -4336,6 +4365,174 @@ public SSHKeyPair registerSSHKeyPair(final RegisterSSHKeyPairCmd cmd) { return createAndSaveSSHKeyPair(name, fingerprint, publicKey, null, owner); } + @Override + public boolean deleteUserData(final DeleteUserDataCmd cmd) { Review Comment: would it be better to have `UserDataManager` to manage userdata actions ? ########## server/src/main/java/com/cloud/network/router/CommandSetupHelper.java: ########## @@ -1223,6 +1225,20 @@ private VmDataCommand generateVmDataCommand(final VirtualRouter router, final St return cmd; } + protected void addUserDataDetailsToCommand(VmDataCommand cmd, String userDataDetails) { Review Comment: it looks this method can also be used in `server/src/main/java/com/cloud/network/element/CloudZonesNetworkElement.java` ########## server/src/main/java/com/cloud/network/NetworkModelImpl.java: ########## @@ -2562,6 +2562,18 @@ public List<String[]> generateVmData(String userData, String serviceOffering, lo vmData.add(new String[]{METATDATA_DIR, LOCAL_HOSTNAME_FILE, StringUtils.unicodeEscape(vmHostName)}); vmData.add(new String[]{METATDATA_DIR, LOCAL_IPV4_FILE, guestIpAddress}); + if(userDataDetails != null && !userDataDetails.isEmpty()) { + userDataDetails = userDataDetails.substring(1, userDataDetails.length()-1); + String[] keyValuePairs = userDataDetails.split(","); + for(String pair : keyValuePairs) + { + String[] entry = pair.split("="); + String key = entry[0].trim(); + String value = entry[1].trim(); + vmData.add(new String[]{METATDATA_DIR, key, StringUtils.unicodeEscape(value)}); Review Comment: I think some `key` are reserved, for example SERVICE_OFFERING_FILE, AVAILABILITY_ZONE_FILE, LOCAL_HOSTNAME_FILE, LOCAL_IPV4_FILE ########## server/src/main/java/com/cloud/server/ManagementServerImpl.java: ########## @@ -4336,6 +4365,174 @@ public SSHKeyPair registerSSHKeyPair(final RegisterSSHKeyPairCmd cmd) { return createAndSaveSSHKeyPair(name, fingerprint, publicKey, null, owner); } + @Override + public boolean deleteUserData(final DeleteUserDataCmd cmd) { + final Account caller = getCaller(); + final String accountName = cmd.getAccountName(); + final Long domainId = cmd.getDomainId(); + final Long projectId = cmd.getProjectId(); + + Account owner = null; + try { + owner = _accountMgr.finalizeOwner(caller, accountName, domainId, projectId); + } catch (InvalidParameterValueException ex) { + if (caller.getType() == Account.Type.ADMIN && accountName != null && domainId != null) { + owner = _accountDao.findAccountIncludingRemoved(accountName, domainId); + } + if (owner == null) { + throw ex; + } + } + + final UserDataVO userData = _userDataDao.findById(cmd.getId()); + if (userData == null) { + final InvalidParameterValueException ex = new InvalidParameterValueException( + "A UserData with id '" + cmd.getId() + "' does not exist for account " + owner.getAccountName() + " in specified domain id"); + final DomainVO domain = ApiDBUtils.findDomainById(owner.getDomainId()); + String domainUuid = String.valueOf(owner.getDomainId()); + if (domain != null) { + domainUuid = domain.getUuid(); + } + ex.addProxyObject(domainUuid, "domainId"); + throw ex; + } + + List<VMTemplateVO> templatesLinkedToUserData = _templateDao.findTemplatesLinkedToUserdata(userData.getId()); + if (CollectionUtils.isNotEmpty(templatesLinkedToUserData)) { + throw new CloudRuntimeException(String.format("Userdata %s cannot be removed as it is linked to active template/templates", userData.getName())); + } + + annotationDao.removeByEntityType(AnnotationService.EntityType.USER_DATA.name(), userData.getUuid()); + + return _userDataDao.expunge(userData.getId()); + } + + @Override + public Pair<List<? extends UserData>, Integer> listUserDatas(final ListUserDataCmd cmd) { + final Long id = cmd.getId(); + final String name = cmd.getName(); + final String keyword = cmd.getKeyword(); + + final Account caller = getCaller(); + final List<Long> permittedAccounts = new ArrayList<Long>(); + + final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(cmd.getDomainId(), cmd.isRecursive(), null); + _accountMgr.buildACLSearchParameters(caller, null, cmd.getAccountName(), cmd.getProjectId(), permittedAccounts, domainIdRecursiveListProject, cmd.listAll(), false); + final Long domainId = domainIdRecursiveListProject.first(); + final Boolean isRecursive = domainIdRecursiveListProject.second(); + final ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + final SearchBuilder<UserDataVO> sb = _userDataDao.createSearchBuilder(); + _accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + final Filter searchFilter = new Filter(UserDataVO.class, "id", false, cmd.getStartIndex(), cmd.getPageSizeVal()); + + final SearchCriteria<UserDataVO> sc = sb.create(); + _accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + + if (id != null) { + sc.addAnd("id", SearchCriteria.Op.EQ, id); + } + + if (name != null) { + sc.addAnd("name", SearchCriteria.Op.EQ, name); + } + + if (keyword != null) { + sc.addAnd("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + } + + final Pair<List<UserDataVO>, Integer> result = _userDataDao.searchAndCount(sc, searchFilter); + return new Pair<List<? extends UserData>, Integer>(result.first(), result.second()); + } + + @Override + @ActionEvent(eventType = EventTypes.EVENT_REGISTER_USER_DATA, eventDescription = "registering userdata", async = true) + public UserData registerUserData(final RegisterUserDataCmd cmd) { + final Account owner = getOwner(cmd); + checkForUserDataByName(cmd, owner); + checkForUserData(cmd, owner); + + final String name = cmd.getName(); + String userdata = cmd.getUserData(); + final String params = cmd.getParams(); + + userdata = validateUserData(userdata, cmd.getHttpMethod()); + + return createAndSaveUserData(name, userdata, params, owner); + } + + private String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) { + byte[] decodedUserData = null; + if (userData != null) { + + if (userData.contains("%")) { + try { + userData = URLDecoder.decode(userData, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new InvalidParameterValueException("Url decoding of userdata failed."); + } + } + + if (!Base64.isBase64(userData)) { + throw new InvalidParameterValueException("User data is not base64 encoded"); + } + // If GET, use 4K. If POST, support up to 1M. + if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) { + if (userData.length() >= MAX_HTTP_GET_LENGTH) { + throw new InvalidParameterValueException("User data is too long for an http GET request"); + } + if (userData.length() > VM_USERDATA_MAX_LENGTH.value()) { + throw new InvalidParameterValueException("User data has exceeded configurable max length : " + VM_USERDATA_MAX_LENGTH.value()); + } + decodedUserData = Base64.decodeBase64(userData.getBytes()); + if (decodedUserData.length > MAX_HTTP_GET_LENGTH) { + throw new InvalidParameterValueException("User data is too long for GET request"); + } + } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) { + if (userData.length() >= MAX_HTTP_POST_LENGTH) { + throw new InvalidParameterValueException("User data is too long for an http POST request"); + } + if (userData.length() > VM_USERDATA_MAX_LENGTH.value()) { + throw new InvalidParameterValueException("User data has exceeded configurable max length : " + VM_USERDATA_MAX_LENGTH.value()); + } + decodedUserData = Base64.decodeBase64(userData.getBytes()); + if (decodedUserData.length > MAX_HTTP_POST_LENGTH) { + throw new InvalidParameterValueException("User data is too long for POST request"); + } + } + + if (decodedUserData == null || decodedUserData.length < 1) { + throw new InvalidParameterValueException("User data is too short"); + } + // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR. + return Base64.encodeBase64String(decodedUserData); + } + return null; + } + + /** + * @param cmd + * @param owner + * @throws InvalidParameterValueException + */ + private void checkForUserData(final RegisterUserDataCmd cmd, final Account owner) throws InvalidParameterValueException { + final UserDataVO userData = _userDataDao.findByUserData(owner.getAccountId(), owner.getDomainId(), cmd.getUserData()); + if (userData != null) { + throw new InvalidParameterValueException("A userdata with same content already exists for this account."); Review Comment: it would be good to add the userdata name or id in the exception. ########## server/src/main/java/com/cloud/server/ManagementServerImpl.java: ########## @@ -4336,6 +4365,174 @@ public SSHKeyPair registerSSHKeyPair(final RegisterSSHKeyPairCmd cmd) { return createAndSaveSSHKeyPair(name, fingerprint, publicKey, null, owner); } + @Override + public boolean deleteUserData(final DeleteUserDataCmd cmd) { + final Account caller = getCaller(); + final String accountName = cmd.getAccountName(); + final Long domainId = cmd.getDomainId(); + final Long projectId = cmd.getProjectId(); + + Account owner = null; + try { + owner = _accountMgr.finalizeOwner(caller, accountName, domainId, projectId); + } catch (InvalidParameterValueException ex) { + if (caller.getType() == Account.Type.ADMIN && accountName != null && domainId != null) { + owner = _accountDao.findAccountIncludingRemoved(accountName, domainId); + } + if (owner == null) { + throw ex; + } + } + + final UserDataVO userData = _userDataDao.findById(cmd.getId()); + if (userData == null) { + final InvalidParameterValueException ex = new InvalidParameterValueException( + "A UserData with id '" + cmd.getId() + "' does not exist for account " + owner.getAccountName() + " in specified domain id"); + final DomainVO domain = ApiDBUtils.findDomainById(owner.getDomainId()); + String domainUuid = String.valueOf(owner.getDomainId()); + if (domain != null) { + domainUuid = domain.getUuid(); + } + ex.addProxyObject(domainUuid, "domainId"); + throw ex; + } + + List<VMTemplateVO> templatesLinkedToUserData = _templateDao.findTemplatesLinkedToUserdata(userData.getId()); + if (CollectionUtils.isNotEmpty(templatesLinkedToUserData)) { + throw new CloudRuntimeException(String.format("Userdata %s cannot be removed as it is linked to active template/templates", userData.getName())); + } + + annotationDao.removeByEntityType(AnnotationService.EntityType.USER_DATA.name(), userData.getUuid()); + + return _userDataDao.expunge(userData.getId()); Review Comment: why not use `remove` ? ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: + if (userDataId != null) { + if (userData != null) { + s_logger.info("Both userdata and userdata ID are provided, precedence goes to userdata ID"); Review Comment: would it be better to throw an exception if both userdata and userdataId are provided ? ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: Review Comment: better to use `UserDataOverridePolicy.denyoverride" ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: + if (userDataId != null) { + if (userData != null) { + s_logger.info("Both userdata and userdata ID are provided, precedence goes to userdata ID"); + } + UserData apiUserDataVO = _userDataDao.findById(userDataId); + return apiUserDataVO.getUserData(); + } else if (userData != null) { + return userData; + } else { + UserData templateUserDataVO = _userDataDao.findById(template.getUserDataId()); + if (templateUserDataVO == null) { + String msg = String.format("UserData linked to the template %s is not found", template.getName()); + throw new CloudRuntimeException(msg); + } + return templateUserDataVO.getUserData(); + } + case append: + UserData templateUserDataVO = _userDataDao.findById(template.getUserDataId()); + if (templateUserDataVO == null) { + String msg = String.format("UserData linked to the template %s is not found", template.getName()); + throw new CloudRuntimeException(msg); + } + if (userDataId != null) { + if (userData != null) { Review Comment: use `StringUtils.isNotEmpty`? ########## ui/src/config/section/compute.js: ########## @@ -633,6 +635,77 @@ export default { } ] }, + { + name: 'userdata', + title: 'label.user.data', + icon: 'solution-outlined', + docHelp: 'adminguide/virtual_machines.html#using-ssh-keys-for-authentication', + permission: ['listUserData'], + columns: () => { + var fields = ['name', 'id'] + if (['Admin', 'DomainAdmin'].includes(store.getters.userInfo.roletype)) { + fields.push('account') + } + return fields + }, + resourceType: 'UserData', + details: ['id', 'name', 'userdata', 'account', 'domain', 'params'], + related: [{ + name: 'vm', + title: 'label.instances', + param: 'userdata' + }], + tabs: [ + { + name: 'details', + component: shallowRef(defineAsyncComponent(() => import('@/components/view/DetailsTab.vue'))) + }, + { + name: 'comments', Review Comment: `Comments` tab seems not working ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: Review Comment: better to use `UserDataOverridePolicy.allowoverride" ########## server/src/main/java/com/cloud/vm/UserVmManagerImpl.java: ########## @@ -5597,6 +5616,79 @@ public HypervisorType getHypervisorTypeOfUserVM(long vmId) { return userVm.getHypervisorType(); } + protected String finalizeUserData(String userData, Long userDataId, VirtualMachineTemplate template) { + if (StringUtils.isEmpty(userData) && userDataId == null && (template == null || template.getUserDataId() == null)) { + return null; + } + if (template != null && template.getUserDataId() != null) { + switch (template.getUserDataOverridePolicy()) { + case denyoverride: + if (StringUtils.isNotEmpty(userData) || userDataId != null) { + String msg = String.format("UserData input is not allowed here since template %s is configured to deny any userdata", template.getName()); + throw new CloudRuntimeException(msg); + } + case allowoverride: + if (userDataId != null) { + if (userData != null) { + s_logger.info("Both userdata and userdata ID are provided, precedence goes to userdata ID"); + } + UserData apiUserDataVO = _userDataDao.findById(userDataId); + return apiUserDataVO.getUserData(); + } else if (userData != null) { + return userData; + } else { + UserData templateUserDataVO = _userDataDao.findById(template.getUserDataId()); + if (templateUserDataVO == null) { + String msg = String.format("UserData linked to the template %s is not found", template.getName()); + throw new CloudRuntimeException(msg); + } + return templateUserDataVO.getUserData(); + } + case append: + UserData templateUserDataVO = _userDataDao.findById(template.getUserDataId()); + if (templateUserDataVO == null) { + String msg = String.format("UserData linked to the template %s is not found", template.getName()); + throw new CloudRuntimeException(msg); + } + if (userDataId != null) { + if (userData != null) { + s_logger.info("Both userdata and userdata ID are provided, precedence goes to userdata ID"); Review Comment: throw an exception ? ########## api/src/main/java/org/apache/cloudstack/api/command/user/userdata/LinkUserDataToTemplateCmd.java: ########## @@ -0,0 +1,118 @@ +// 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.user.userdata; + +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.user.Account; +import com.cloud.user.UserData; +import com.cloud.utils.exception.CloudRuntimeException; +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.ResponseObject; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.AdminCmd; +import org.apache.cloudstack.api.response.TemplateResponse; +import org.apache.cloudstack.api.response.UserDataResponse; +import org.apache.log4j.Logger; + +@APICommand(name = "linkUserDataToTemplate", description = "Links a userdata to a template.", responseObject = TemplateResponse.class, responseView = ResponseObject.ResponseView.Restricted, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class LinkUserDataToTemplateCmd extends BaseCmd implements AdminCmd { + public static final Logger s_logger = Logger.getLogger(LinkUserDataToTemplateCmd.class.getName()); + + private static final String s_name = "linkuserdatatotemplateresponse"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.TEMPLATE_ID, + type = CommandType.UUID, + entityType = TemplateResponse.class, + required = true, + description = "the ID of the template for the virtual machine") + private Long templateId; + + @Parameter(name = ApiConstants.USER_DATA_ID, + type = CommandType.UUID, + entityType = UserDataResponse.class, + required = true, Review Comment: we cannot remove the link the template from the userdata, right ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
