Passing domain id without an account ignored #7776

soreana · 2023-07-26T15:09:13Z

ISSUE TYPE

Bug Report

COMPONENT NAME

API

CLOUDSTACK VERSION

4.17, 4.18, main, and probably most of the previous versions

SUMMARY

When the API calls are made using only the domainid parameter, CloudStack currently ignores it. For instance, in the createNetwork api call, there is an option to pass both domainid and account parameters. All combinations of domainid and account in the following table make sense except for the second combination. As a user, I would expect that the network should be created under the specified domain or receive an error message stating that I need to provide an account as well. However, nowhere in the API documentation does it mention that an account must be provided when calling createNetwork with a specific domainid.

idx	domainid parameter is in API call	account parameter is in API call	result
1	yes	yes	network created under the account and domain
2	yes	NO	network created under the caller domain and account
3	NO	yes	network creation failed
4	NO	NO	network created under the caller domain and account

STEPS TO REPRODUCE

Run the following command to create a network (type of network doesn't matter, I also checked the deploy virtual machine it is the same behaviour)

(localcloud) > create network domainid=<domain id> name=L2-withou-account displaytext=L2-without-account zoneid=<zone id> networkofferingid=<offering id>
{
  "network": {
    "account": "admin",
    "acltype": "Account",
...
    "domain": "ROOT",
    "domainid": "2ddf37db-1fc3-11ee-a012-1e00cf003378",
  }
}

EXPECTED RESULTS

CloudStack throws an error or creates the network under the correct domain

ACTUAL RESULTS

Network created under the root domain.

The text was updated successfully, but these errors were encountered:

weizhouapache · 2023-07-27T01:52:58Z

@soreana
this is expected behavior, I think

To create a network for a domain , you need to pass acltype=Domain
refer to https://cloudstack.apache.org/api/apidocs-4.18/apis/createNetwork.html

harikrishna-patnala · 2023-07-27T04:20:21Z

Yes @soreana this is expected behaviour.

To keep the better visibility wrt to accounts, last time we have fixed the parameter descriptions (#7387) and keep a drop down of accounts under the selected domain (#7393) in 4.18.1

DaanHoogland · 2023-07-27T07:05:13Z

@weizhouapache @harikrishna-patnala , @soreana states, 'or receive an error message stating that I need to provide an account as well.' Does this make sense to you?

If the user provides a domainid it is strange to create the network in another domain, even when only providing a domainid is not a valid parameter set. (imnsho)

harikrishna-patnala · 2023-07-27T07:21:29Z

@DaanHoogland yes error can be thrown, I think this is a design thought. Last time I just logged a message, didn't want to break the backward compatibility.

cloudstack/server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines 1405 to 1406 in 41bbedb

    
           s_logger.info(String.format("Assigning the network to caller:%s because either projectId or accountname and domainId are not provided", caller.getAccountName())); 
        
           owner = caller;

DaanHoogland · 2023-07-27T07:31:15Z

@DaanHoogland yes error can be thrown, I think this is a design thought. Last time I just logged a message, didn't want to break the backward compatibility.

cloudstack/server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines 1405 to 1406 in 41bbedb

s_logger.info(String.format("Assigning the network to caller:%s because either projectId or accountname and domainId are not provided", caller.getAccountName()));

owner = caller;

that message is an info in case "either projectId or accountname and domainId are not provided". I think we can split between that case and when domainid is provided but projectId or accountname are not. This is a minor issue, but it makes sense to return an error in those cases. (if we all agree!)

weizhouapache · 2023-07-27T07:41:33Z

@DaanHoogland yes error can be thrown, I think this is a design thought. Last time I just logged a message, didn't want to break the backward compatibility.

cloudstack/server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines 1405 to 1406 in 41bbedb

s_logger.info(String.format("Assigning the network to caller:%s because either projectId or accountname and domainId are not provided", caller.getAccountName()));

owner = caller;

that message is an info in case "either projectId or accountname and domainId are not provided". I think we can split between that case and when domainid is provided but projectId or accountname are not. This is a minor issue, but it makes sense to return an error in those cases. (if we all agree!)

@DaanHoogland
yes, thrown an error message if

acltype is Account, and
domainid is passed, and
account name and projectid are not passed

in my opinion, there will be more trouble than benefit.
(the behaviour has existed for many years)

DaanHoogland · 2023-07-27T08:34:30Z

@DaanHoogland yes error can be thrown, I think this is a design thought. Last time I just logged a message, didn't want to break the backward compatibility.

cloudstack/server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines 1405 to 1406 in 41bbedb

s_logger.info(String.format("Assigning the network to caller:%s because either projectId or accountname and domainId are not provided", caller.getAccountName()));

owner = caller;

that message is an info in case "either projectId or accountname and domainId are not provided". I think we can split between that case and when domainid is provided but projectId or accountname are not. This is a minor issue, but it makes sense to return an error in those cases. (if we all agree!)

@DaanHoogland yes, thrown an error message if
* acltype is Account, and

* domainid is passed, and

* account name and projectid are not passed
in my opinion, there will be more trouble than benefit. (the behaviour has existed for many years)

ok, agree @soreana ?

soreana · 2023-07-27T11:56:13Z

@DaanHoogland yes error can be thrown, I think this is a design thought. Last time I just logged a message, didn't want to break the backward compatibility.

cloudstack/server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines 1405 to 1406 in 41bbedb

s_logger.info(String.format("Assigning the network to caller:%s because either projectId or accountname and domainId are not provided", caller.getAccountName()));

owner = caller;

that message is an info in case "either projectId or accountname and domainId are not provided". I think we can split between that case and when domainid is provided but projectId or accountname are not. This is a minor issue, but it makes sense to return an error in those cases. (if we all agree!)

@DaanHoogland yes, thrown an error message if
* acltype is Account, and

* domainid is passed, and

* account name and projectid are not passed
in my opinion, there will be more trouble than benefit. (the behaviour has existed for many years)
ok, agree @soreana ?

@DaanHoogland Yeah, that is makes sense, CloudStack should throw an error when only the domain id is provided.

Btw, @DaanHoogland @harikrishna-patnala @weizhouapache the createNetwork API call was a tip of an ice burg, even if we make it works other API calls still have an issue, I've tested a few (deployVirtualmachine, createSnapshot, createVolume ) they are all have the same issue ...

(localcloud)  > deploy virtualmachine  domainid=<domain id>  networkids=<network id> serviceofferingid=<service offering id> templateid=<template id> zoneid=<zone id>
{
  "virtualmachine": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}
(localcloud)  > create snapshot domainid=<domain id> volumeid=<volume id>
{
  "snapshot": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
..
  }
}
(localcloud)  > create volume  domainid=<domain id> snapshotid=<snapshot id>
{
  "volume": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}

weizhouapache · 2023-07-27T13:49:14Z

ok, agree @soreana ?

@DaanHoogland Yeah, that is makes sense, CloudStack should throw an error when only the domain id is provided.

Btw, @DaanHoogland @harikrishna-patnala @weizhouapache the createNetwork API call was a tip of an ice burg, even if we make it works other API calls still have an issue, I've tested a few (deployVirtualmachine, createSnapshot, createVolume ) they are all have the same issue ...
(localcloud)  > deploy virtualmachine  domainid=<domain id>  networkids=<network id> serviceofferingid=<service offering id> templateid=<template id> zoneid=<zone id>
{
  "virtualmachine": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}
(localcloud)  > create snapshot domainid=<domain id> volumeid=<volume id>
{
  "snapshot": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
..
  }
}
(localcloud)  > create volume  domainid=<domain id> snapshotid=<snapshot id>
{
  "volume": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}

@soreana
yes
I prefer not to change any of them. We do not know what the changes might cause.
ui, cmk, users' scripts, terraform, etc

soreana · 2023-07-27T15:10:58Z

ok, agree @soreana ?

@DaanHoogland Yeah, that is makes sense, CloudStack should throw an error when only the domain id is provided.
Btw, @DaanHoogland @harikrishna-patnala @weizhouapache the createNetwork API call was a tip of an ice burg, even if we make it works other API calls still have an issue, I've tested a few (deployVirtualmachine, createSnapshot, createVolume ) they are all have the same issue ...
(localcloud)  > deploy virtualmachine  domainid=<domain id>  networkids=<network id> serviceofferingid=<service offering id> templateid=<template id> zoneid=<zone id>
{
  "virtualmachine": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}
(localcloud)  > create snapshot domainid=<domain id> volumeid=<volume id>
{
  "snapshot": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
..
  }
}
(localcloud)  > create volume  domainid=<domain id> snapshotid=<snapshot id>
{
  "volume": {
    "account": "admin",
    "domain": "ROOT",
    "domainid": "ea2dcd59-249f-11ee-bb7d-1e000d003377",
...
  }
}
@soreana yes I prefer not to change any of them. We do not know what the changes might cause. ui, cmk, users' scripts, terraform, etc

@weizhouapache That is a valid concern. Can We at least change the API documentations?

weizhouapache · 2023-07-28T01:50:53Z

@weizhouapache That is a valid concern. Can We at least change the API documentations?

@soreana
yes, we can change the description of API parameters, and UI. These have small impact.
Would you like to make the changes ?

harikrishna-patnala · 2023-07-28T03:07:50Z

In a way I agree with both @soreana and @weizhouapache as this is a valid concern but fixing this may raise immediate issues with the existing environments.

May be we can do below things

Fix API parameters descriptions
Make account selection mandatory when domain is selected only in UI. API will continue to work as is.

I'm not sure how hard is to make point 2 working in UI, if that takes time we can go with point 1 only.

weizhouapache · 2023-07-28T03:15:46Z

with the existing environments.

May be we can do below things

Fix API parameters descriptions

Make account selection mandatory when domain is selected only in UI. API will continue to work as is.

I'm not sure how hard is to make point 2 working in UI, if that takes time we can go with point 1 only.

agree with @harikrishna-patnala

DaanHoogland · 2023-07-28T07:45:17Z

with the existing environments.
May be we can do below things

Fix API parameters descriptions

Make account selection mandatory when domain is selected only in UI. API will continue to work as is.

I'm not sure how hard is to make point 2 working in UI, if that takes time we can go with point 1 only.

agree with @harikrishna-patnala

I am not so keen in making implementing functionality in the UI only. it creates an inconsistency for users and add a maintenance burdon on developers.

DaanHoogland · 2023-08-04T07:28:50Z

so far I see the following for the create network API

    @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "domain ID of the account owning a network. " +
            "If no account is provided then network will be assigned to the caller account and domain")
    private Long domainId;

this looks like enough documentation to me. cc @soreana @harikrishna-patnala @weizhouapache

Are there other places I should look?

soreana · 2023-08-04T18:30:01Z

so far I see the following for the create network API
    @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "domain ID of the account owning a network. " +
            "If no account is provided then network will be assigned to the caller account and domain")
    private Long domainId;
this looks like enough documentation to me. cc @soreana @harikrishna-patnala @weizhouapache

Are there other places I should look?

@DaanHoogland That would be enough, I will take care of them.

weizhouapache · 2023-08-14T08:07:59Z

@DaanHoogland That would be enough, I will take care of them.

@soreana
any update ?

soreana · 2023-08-16T15:27:47Z

@DaanHoogland That would be enough, I will take care of them.

@soreana any update ?

I've created #7876 PR, let me know if other API calls needs to be added.

soreana · 2023-08-20T14:09:29Z

Closing this issue as the #7876 got merged.

soreana added type:bug component:api Severity:Minor labels Jul 26, 2023

DaanHoogland added this to the 4.18.1.0 milestone Aug 4, 2023

soreana mentioned this issue Aug 4, 2023

Fix spelling apache/cloudstack-go#64

Merged

soreana self-assigned this Aug 4, 2023

soreana mentioned this issue Aug 16, 2023

Api: update command domainId/account descriptions #7876

Merged

12 tasks

soreana closed this as completed Aug 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Passing domain id without an account ignored #7776

Passing domain id without an account ignored #7776

soreana commented Jul 26, 2023

weizhouapache commented Jul 27, 2023

harikrishna-patnala commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

harikrishna-patnala commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

weizhouapache commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

soreana commented Jul 27, 2023 •

edited

Loading

weizhouapache commented Jul 27, 2023

soreana commented Jul 27, 2023 •

edited

Loading

weizhouapache commented Jul 28, 2023

harikrishna-patnala commented Jul 28, 2023 •

edited

Loading

weizhouapache commented Jul 28, 2023

DaanHoogland commented Jul 28, 2023

DaanHoogland commented Aug 4, 2023

soreana commented Aug 4, 2023

weizhouapache commented Aug 14, 2023

soreana commented Aug 16, 2023

soreana commented Aug 20, 2023

Passing domain id without an account ignored #7776

Passing domain id without an account ignored #7776

Comments

soreana commented Jul 26, 2023

ISSUE TYPE

COMPONENT NAME

CLOUDSTACK VERSION

SUMMARY

STEPS TO REPRODUCE

EXPECTED RESULTS

ACTUAL RESULTS

weizhouapache commented Jul 27, 2023

harikrishna-patnala commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

harikrishna-patnala commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

weizhouapache commented Jul 27, 2023

DaanHoogland commented Jul 27, 2023

soreana commented Jul 27, 2023 • edited Loading

weizhouapache commented Jul 27, 2023

soreana commented Jul 27, 2023 • edited Loading

weizhouapache commented Jul 28, 2023

harikrishna-patnala commented Jul 28, 2023 • edited Loading

weizhouapache commented Jul 28, 2023

DaanHoogland commented Jul 28, 2023

DaanHoogland commented Aug 4, 2023

soreana commented Aug 4, 2023

weizhouapache commented Aug 14, 2023

soreana commented Aug 16, 2023

soreana commented Aug 20, 2023

soreana commented Jul 27, 2023 •

edited

Loading

soreana commented Jul 27, 2023 •

edited

Loading

harikrishna-patnala commented Jul 28, 2023 •

edited

Loading