You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/03 08:46:38 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4200: Allow domain admins to create offering without mentioning domainid
ravening opened a new pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200
## Description
<!--- Describe your changes in detail -->
Part 1:
While creating disk/service offering by domains we need to
specify domain id else it wont be created. Provide an enhancement
so that we dont need to pass domainid and it will be taken
from the callers user similar to other api's
Another bug fix
Part2:
Currently while creating any offering, we need to
specify a domain id or a list of domain id.
If an offering is created on a domain then all its child
domains should see it and all the parents domain
should see it as well but it should not be visible
to sibling domain
isrecursive=true ensures that all child domains can see it
but if a service offering is created for the domain at the leaf
of the domain path then its parent cant see it if isrecurise
is true. so we need to pass isrecursive=false so that the
parent domain can see the child offerings
### Cloudmonkey works fine but the issue is only with UI
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
Without the UI fix
After the UI fix
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
This has been tested both from cloudmonkey for part1 and part2 and UI for part2
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
Part 1
Cloudmonkey api before fix
```
(test1) 🐵 > create serviceoffering name=without-domainid displaytext=without-domaindi
Error 431: Unable to create public service offering by admin: cff2f496-0ab1-4aa9-8447-e89be9e42b10 because it is domain-admin
(test1) 🐵 > create diskoffering name=without-domain displaytext=without-domain disksize=10
Error 431: Unable to create public disk offering by admin: cff2f496-0ab1-4aa9-8447-e89be9e42b10 because it is domain-admin
(test1) 🐵 >
```
After the fix
```
(test1) 🐵 > list domains filter=name
{
"count": 1,
"domain": [
{
"name": "test1"
}
]
}
(test1) 🐵 > create serviceoffering name=without-domain displaytext=without-domain
{
"serviceoffering": {
"created": "2020-07-02T13:17:39+0000",
"defaultuse": false,
"displaytext": "without-domain",
"domain": "test1",
"domainid": "c14ffcea-e197-4d6f-b8ff-2ffb854cd443",
"id": "e7bb6b10-060b-4d8a-aec2-831691e868ac",
"iscustomized": true,
"issystem": false,
"isvolatile": false,
"limitcpuuse": false,
"name": "without-domain",
"offerha": false,
"provisioningtype": "thin",
"serviceofferingdetails": {
"domainid": "2"
},
"storagetype": "shared"
}
}
(test1) 🐵 > create diskoffering name=without-domain displaytext=without-domain disksize=10
{
"diskoffering": {
"created": "2020-07-02T13:18:32+0000",
"disksize": 10,
"displayoffering": true,
"displaytext": "without-domain",
"domain": "/test1/",
"domainid": "c14ffcea-e197-4d6f-b8ff-2ffb854cd443",
"id": "552c9c4a-04af-4d6b-b50d-2dd0cb374668",
"iscustomized": false,
"name": "without-domain",
"provisioningtype": "thin",
"storagetype": "shared"
}
}
```
Part 2
cloudmonkey api working as expected without any changes.
```
(test11) 🐵 > list diskofferings listall=true filter=name isrecursive=true
{
"count": 6,
"diskoffering": [
{
"name": "Small"
},
{
"name": "Medium"
},
{
"name": "Large"
},
{
"name": "Custom"
},
{
"name": "test-11only"
},
{
"name": "test-11andtest2"
}
]
}
```
```
(test11) 🐵 > list diskofferings listall=true filter=name
{
"count": 9,
"diskoffering": [
{
"name": "Small"
},
{
"name": "Medium"
},
{
"name": "Large"
},
{
"name": "Custom"
},
{
"name": "test1only"
},
{
"name": "test-11only"
},
{
"name": "test1-and-11"
},
{
"name": "root-test1-test11"
},
{
"name": "test-11andtest2"
}
]
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-724954312
> > code lgtm, @ravening please make sure there are primate changes as well
>
> @DaanHoogland i think this is only backend change.
@ravening, how about the changes in `ui/scripts/configuration.js`, then?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-739824594
> There is a world of difference between being able to create an offering and being able to create VM **on** that offering. You simply cannot compare them.
>
> I'd like to see a feasible example of why this change is 'OK'
@PaulAngus Just wondering, how creating a vm without passing domainid is ok but creating service offering without domainid is not ok?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-740132818
That's simple.
With service offerings, you both protect the infra and set what the users can do within that infra according to your use case. What anyone can do WITHIN those bounds will be fine, assuming that the bounds were set correctly. Therefore it is essential that the service offerings and their coverage is spot-on. Mistakes there could cause real problems.
You're taking away a safety net and you're not adding something that domain admins couldn't do if they passed the domain id in the first place.
It looks to me that you want to make it easier for admins to get in a car by removing all of the locks.
But it's not worth this amount of energy to keep arguing.
So someone can merge this if they want.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902513544
@weizhouapache clear except,
> @rhtyd See my reply inline:
>
> > @DaanHoogland I suppose either some arguments to answer the concerns - security or billing worries; or a global setting to toggle between the feature. For example, some test cases (either manually tested or a smoketests exists or is added):
> >
...
> > What if I'm domain admin of a sub-domain; can the offering be used by accounts of parent domain?
>
> No. except the other account is ROOT admin.
so if a ROOT admin account uses the offering, can the domain admin no longer delete it? or will it be copied/moved to the admins domain?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-657455000
> Assuming that a domain admin can add offerings to one of their sub-domains, then it makes sense (to me) to ensure that the domain admin is specific. If that's the case, then I am -1 on taking that restriction away (with great power comes great responsibility).
>
> We no longer support the legacy UI, so no change should be accepted for it.
>
> wrt see child or parent offerings, are you referring to what Users or Domain Admins can see?
Its quite similar to deploying a vm. When we login as "domain admin" in cloudmonkey api and deploy a vm, we dont need to pass domainid. The domainid will be taken from caller user id. Same thing I am applying for creating offering.
If we login as domain admin then domain id of that caller should be taken while creating offering
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909103406
Sorry Rakesh, I'm -1 as it may introduce security/operational/access risks and issues, and points raised by others on this PR. I haven't tested or reviewed it though.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-726933313
@PaulAngus can you please elaborate more on the risks involved on domain admin deploying VMs on its own domain by default?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909090346
@rhtyd @PaulAngus @GabrielBrascher @weizhouapache @RodrigoDLopez @DaanHoogland
we can quickly have either a +1 or -1 without further discussion and depending on that I will either close the pr or update the description of the pr
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-724945169
> code lgtm, @ravening please make sure there are primate changes as well
@DaanHoogland i think this is only backend change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-863950713
We have enough :+1: but also worries here. Only from @rhtyd and @PaulAngus , i think. Are these still valid and what is needed?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-712788045
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-804999576
Thanks for fixing the conflicts @ravening.
## Here are my two cents:
From what I understood, the main discussion has been due to confusion around the scope and goals of this PR; I would like to bring some consensus and align the goals and expectations so we all _speak the same language_.
**Note:** this feedback is based on what I understood out of a quick check on all comments. Please let me know if I am missing some points here.
### Main conserns
If I understood it correctly, the main issue raised for this PR was regarding the fact that Domain admins **cannot** create public offerings.
I totally understand and agree with @PaulAngus' concerns; we need to keep the behavior of **Domain admins** creating only **non-public** offerings. If that was not the case then we would need to escalate to a deeper discussion to decide whether to change such behavior on the API or not. However, I think that this PR properly avoids the creation of public offerings by domain admins.
### Conclusion
I am still **+1** on having this merged, I don't see any harm in domain admins setting offerings to its domain, by default. When a ROOT admin creates an offering it is already set on its own domain. The code and automated/manual tests are aligned with the description.
The only point that may need some attention is regarding the documentation. According to the [API documentation](https://cloudstack.apache.org/api/apidocs-4.15/apis/createServiceOffering.html) a NULL _domainid_ results in a public offering, which is not the case of domain admins; on the other side, it is not a required parameter.
> domainid: the ID of the containing domain(s), null for public offerings`
#### Pros:
1. personally I think it enhances the user experience, adding flexibility and avoiding "unnecessary" error messages
2. I myself expected _domain id_ to not be required due to the API description
2. it keeps the offerings **NOT** public for domain-admins, regardless of the _domainid_
3. offerings created by the domain admins are still under his/her own domain
4. domain admins can still create offerings for sub-domains by adding the sub-domain ID
#### Cons:
1. We might need to update the API description by stating the behavior when domain-admins keep it NULL
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-804960153
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 214
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902601973
> there is another example, when create a shared network by root admin (networks are more important than service offerings, right ?), acltype and domain/account are optional not required. if root admin forgets to pass acltype, the created network will be visible by all users. is it more risky ?
>
So from your statement, we agree that not requiring admins to be specific is risky. So then remove the _risk_ from shared network creation. Don't add risk to service offering creation.
> if a change has no risk but make life a little bit easier, why not accept it ?
As your shared network example demonstrates, it **does** add risk. The upside seems minuscule to me. So why the desperation to do it?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902568511
> Thanks for fixing the conflicts @ravening.
>
> ## Here are my two cents:
> From what I understood, the main discussion has been due to confusion around the scope and goals of this PR; I would like to bring some consensus and align the goals and expectations so we all _speak the same language_.
>
> **Note:** this feedback is based on what I understood out of a quick check on all comments. Please let me know if I am missing some points here.
>
> ### Main conserns
> If I understood it correctly, the main issue raised for this PR was regarding the fact that Domain admins **cannot** create public offerings.
>
> I totally understand and agree with @PaulAngus' concerns; we need to keep the behavior of **Domain admins** creating only **non-public** offerings. If that was not the case then we would need to escalate to a deeper discussion to decide whether to change such behavior on the API or not. However, I think that this PR properly avoids the creation of public offerings by domain admins.
>
> ### Conclusion
> I am still **+1** on having this merged, I don't see any harm in domain admins setting offerings to its domain, by default. When a ROOT admin creates an offering it is already set on its own domain. The code and automated/manual tests are aligned with the description.
>
> The only point that may need some attention is regarding the documentation. According to the [API documentation](https://cloudstack.apache.org/api/apidocs-4.15/apis/createServiceOffering.html) a NULL _domainid_ results in a public offering, which is not the case of domain admins; on the other side, it is not a required parameter.
>
> > domainid: the ID of the containing domain(s), null for public offerings`
>
> #### Pros:
> 1. personally I think it enhances the user experience, adding flexibility and avoiding "unnecessary" error messages
> 2. I myself expected _domain id_ to not be required due to the API description
> 3. it keeps the offerings **NOT** public for domain-admins, regardless of the _domainid_
> 4. offerings created by the domain admins are still under his/her own domain
> 5. domain admins can still create offerings for sub-domains by adding the sub-domain ID
>
> #### Cons:
> 1. We might need to update the API description by stating the behavior when domain-admins keep it NULL
@ravening could you update api descrition as suggested by @GabrielBrascher ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-653745135
> Should domain admins be allowed to create public offerings? They must be restricted only to their domains I think.
Domain admins cant create public offerings. If they create offering, it will be only visible to their and their subdomains.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-740534422
> That's simple.
> With service offerings, you both protect the infra and set what the users can do within that infra according to your use case. What anyone can do WITHIN those bounds will be fine, assuming that the bounds were set correctly. Therefore it is essential that the service offerings and their coverage is spot-on. Mistakes there could cause real problems.
>
> You're taking away a safety net and you're not adding something that domain admins couldn't do if they passed the domain id in the first place.
> It looks to me that you want to make it easier for admins to get in a car by removing all of the locks.
>
> But it's not worth this amount of energy to keep arguing.
> So someone can merge this if they want.
@PaulAngus
Hi Paul, it is not a car :-D
it is just a service offering, not vms, volumes, templates, networks.
in my opinion, there is no any risk if domain admin create service offering for its domain without domainid.
maybe you still remember that, in older cloudstack versions, domain admin can create public offering which can be accessed by everyone (even in other domains).
It is up to you guys if merge it or not. @rhtyd @PaulAngus
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-677712871
@blueorangutan test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-923716631
@nvazquez
let's close this pr ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-726890927
@ravening I am absolutely -1 on allowing admins to screw-up by not requiring the domain id, there is no need. Requiring it does not stop admins from doing anything
isrecursive might be nice though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-713227303
<b>Trillian test result (tid-3022)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37523 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4200-t3022-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 363.66 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 417.54 | test_vpc_redundant.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-712874066
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909222161
@ravening
I'm +1 with this one. At this point, I cannot see any risk or damage caused by this PR. As I said, I don't see the lack of attention as a risk and it's not.
But I think the better solution is close this one. Since it won't go through.
Maybe on an close future you can reopen this discussion, when we have more cases where this flexibility will be appropriate to the context
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-805455688
<b>Trillian test result (tid-239)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39786 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4200-t239-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_migrate_VM_and_root_volume | `Error` | 71.20 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 53.06 | test_vm_life_cycle.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902623331
> negligible benefit.
I think the benefit is commercial, but can't be sure. and yes, oxygen is getting low. put it behind a setting that defaults to false please.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-901716444
linked to #3248
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-901722110
@rhtyd See my reply inline:
> @DaanHoogland I suppose either some arguments to answer the concerns - security or billing worries; or a global setting to toggle between the feature. For example, some test cases (either manually tested or a smoketests exists or is added):
>
> * Can I as a domain admin create an offering without domain ID, but that offering is used by other accounts outside of the domain?
No. except accounts in sub-domains can access the offering.
> What if I'm domain admin of a sub-domain; can the offering be used by accounts of parent domain?
No. except the other account is ROOT admin.
> * What are the scope of the offering/settings that domain admin can add, for ex. can I specify storage tag etc?
>
No. host tags and storage tags can only be set by ROOT admin.
> cc @ravening
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-665579509
> @ravening when creating an offering in the domain admins domain, are those automatically visible in subdomains and in parent domain(-chain)?
it will be visible only for that domain.
>
> > If an offering is created on a domain then all its child
> > domains should see it and all the parents domain
> > should see it as well but
> > ...
>
> > isrecursive=true ensures that all child domains can see it
> > but if a service offering is created for the domain at the leaf
> > of the domain path then its parent cant see it if isrecurise
> > is true. so we need to pass isrecursive=false so that the
> > parent domain can see the child offerings
>
> this sounds contradictory. the isrecursive flag should have an intuitive meaning that is consistent for a 'leave' or a 'node' domain. So whether or not isrecursive is set should have the same consequences in bith domain types.
>
> Am I misunderstanding something?
yeah the `isrecursive` kind of confusing sometimes.
Lets say I have this domain relationship
ROOT -> child1 -> child11
if I try to list offering for domain ROOT by passing `isrecursive=true` then i see all offerings of ROOT, child1 and child11.
Same applies for child1
now if i try to list offerings for domain child11 by passing `isrecursive=true` then it wont list the offering belonging to parent domain `child1` even though it should be displayed
if you dont pass `isrecursive=true` and try listing offerings for child11 then you will see all the parent domain offerings.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-726933313
@PaulAngus can you please elaborate more on the risks involved on domain admin creating offering on its own domain by default?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-653725463
Should domain admins be allowed to create public offerings? They must be restricted only to their domains I think.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-671869938
> > Same applies for child1
>
> I think this is a bug, `isrecursive` should not traverse up, only down.
>
> > if you dont pass isrecursive=true and try listing offerings for child11 then you will see all the parent domain offerings.
@DaanHoogland `isrecursive` is traversing down as expected.
Problem happens when this flag is true and function is called with child domain id as first parameter and parent domain id as second parameter.
If `isrecursive` is true, it checks if the second parameter is the child domain of first parameter. Since we are passing parent domain as the second parameter, the condition will be false and it will never display the offerings belonging to parent domain
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-677705692
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-1795
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902615322
@PaulAngus @weizhouapache , as all other discussion is resolved I think that the way to go is to put this behind a global or domain setting, and let the operator decide. Would that be a good compromise?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland edited a comment on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland edited a comment on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-907033109
> @PaulAngus, @DaanHoogland
> Is there anything else that could be done by @ravening to proceed with this proposal?
@RodrigoDLopez , as you can see this PR already had my approval for a long time. I am just trying to address @PaulAngus' concern, which has some merit. To me it doesn't warrant blocking the PR, though.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-663377814
> nice enhance @ravening
> I ran some manual tests using 2 accounts
>
> * admin-domain that belongs to /domain1
> * admin-subdomain that belongs to /domain1/subdomain1
>
> With the 'admin-domain' account I was able to create offers on 'domain1' without making it explicit, which is the PR proposal
>
> ```
> () 🐱 > create diskoffering name=admindomain displaytext=admindomain customized=true
> {
> "diskoffering": {
> "created": "2020-07-23T20:21:28+0000",
> "disksize": 0,
> "displayoffering": true,
> "displaytext": "admindomain",
> "domain": "/domain1/",
> "domainid": "86efc439-bd1e-4a9e-ac06-aead0c8b7af3",
> "id": "8b7d1b26-9c44-45b7-817d-f52b78a7b7ee",
> "iscustomized": true,
> "name": "admindomain",
> "provisioningtype": "thin",
> "storagetype": "shared"
> }
> }
> ```
>
> works like a charm.
> I tested many list commands, and everything is working as expected.
>
> With the 'admin-subdomain1' account I got the same behavior, I was able to create offers on '/domain1/subdomain1' without making it explicit
>
> ```
> () 🐱 > create diskoffering name=subdomain displaytext=subdomain customized=true
> {
> "diskoffering": {
> "created": "2020-07-23T20:31:58+0000",
> "disksize": 0,
> "displayoffering": true,
> "displaytext": "subdomain",
> "domain": "/domain1/subdomain1/",
> "domainid": "a412bf4c-ab00-448b-9ab9-07b05564402b",
> "id": "0386c751-4e6f-4322-a946-37987b597025",
> "iscustomized": true,
> "name": "subdomain",
> "provisioningtype": "thin",
> "storagetype": "shared"
> }
> }
> ```
>
> code looks good to me, makes it easy to create offers for accounts that are domain admin.
> I am +1 with this PR proposal.
Thanks for testing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-804817405
> @rhtyd moved it to 4.16 and it needs a rebase, so let's discuss further after release, @weizhouapache.
@DaanHoogland @rhtyd @weizhouapache rebased it to latest master and changed primate code as well
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-907087891
to reiterate;
- I'm -0 on this.
- This adds some amount of risk with no real stated justification as far as I can see
- There are over 40 parameters to the createserviceoffering API. Basic offerings are there out of the box so, the domain admin **will** be using a chunk of them to define a 'non-basic' offering, removing the need to specify **one** parameter will make a negligible difference to the effort involved.
Yes, a global setting defaulting to current behaviour would add back the failsafe that is currently in place.
Yes, that does seem like massive overkill, but so is the PR IMO so 🤷🏻♂️.
While this issue has taken center stage, there is still an outstanding question/comment from Gabriel, RE null domainids creating public offerings, which needs to be addressed.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-924251237
Thanks all, closing after the remarks
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-740456178
Given lack of confidence/testing I've moved to 4.16, let's revisit this soon and keep the discussion going. Thanks @ravening
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-663233493
nice enhance @ravening
I ran some manual tests using 2 accounts
* admin-domain that belongs to /domain1
* admin-subdomain that belongs to /domain1/subdomain1
With the 'admin-domain' account I was able to create offers on 'domain1' without making it explicit, which is the PR proposal
```
() 🐱 > create diskoffering name=admindomain displaytext=admindomain customized=true
{
"diskoffering": {
"created": "2020-07-23T20:21:28+0000",
"disksize": 0,
"displayoffering": true,
"displaytext": "admindomain",
"domain": "/domain1/",
"domainid": "86efc439-bd1e-4a9e-ac06-aead0c8b7af3",
"id": "8b7d1b26-9c44-45b7-817d-f52b78a7b7ee",
"iscustomized": true,
"name": "admindomain",
"provisioningtype": "thin",
"storagetype": "shared"
}
}
```
works like a charm.
I tested many list commands, and everything is working as expected.
With the 'admin-subdomain1' account I got the same behavior, I was able to create offers on '/domain1/subdomain1' without making it explicit
```
() 🐱 > create diskoffering name=subdomain displaytext=subdomain customized=true
{
"diskoffering": {
"created": "2020-07-23T20:31:58+0000",
"disksize": 0,
"displayoffering": true,
"displaytext": "subdomain",
"domain": "/domain1/subdomain1/",
"domainid": "a412bf4c-ab00-448b-9ab9-07b05564402b",
"id": "0386c751-4e6f-4322-a946-37987b597025",
"iscustomized": true,
"name": "subdomain",
"provisioningtype": "thin",
"storagetype": "shared"
}
}
```
code looks good to me, makes it easy to create offers for accounts that are domain admin.
I am +1 with this PR proposal.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-725453721
> > > > code lgtm, @ravening please make sure there are primate changes as well
> >
> >
> > >
> >
> >
> > > @DaanHoogland i think this is only backend change.
> >
> >
> > @ravening, how about the changes in `ui/scripts/configuration.js`, then?
>
> @DaanHoogland they are bug fix changes for displaying output in ui. I guess they're not needed for new ui. I will cross verify tomorrow
@DaanHoogland no changes needed for primate. changes look good there
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-712873454
@blueorangutan test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-656603579
Assuming that a domain admin can add offerings to one of their sub-domains, then it makes sense (to me) to ensure that the domain admin is specific. If that's the case, then I am -1 on taking that restriction away (with great power comes great responsibility).
We no longer support the legacy UI, so no change should be accepted for it.
wrt see child or parent offerings, are you referring to what Users or Domain Admins can see?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902608970
> > there is another example, when create a shared network by root admin (networks are more important than service offerings, right ?), acltype and domain/account are optional not required. if root admin forgets to pass acltype, the created network will be visible by all users. is it more risky ?
>
> So from your statement, we agree that not requiring admins to be specific is risky. So then remove the _risk_ from shared network creation. Don't add risk to service offering creation.
>
> > if a change has no risk but make life a little bit easier, why not accept it ?
>
> As your shared network example demonstrates, it **does** add risk. The upside seems minuscule to me. So why the desperation to do it?
@PaulAngus
domain admin can only create service offering which are visible for accounts under his domain. is it risky ? I do not think so.
it is much more risky to create share networks without acltype and we accept the risk. why not accept the change on a much less risky resource (service offering v.s. network) and limited impaction (in domain admin's domain v.s. whole platform) ? I cannot understand, to be honest.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909103406
Sorry Rakesh, I'm -1 as it may introduce security/operational/access risks and issues, and points raised by others on this PR. I haven't tested or reviewed it though.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909090346
@rhtyd @PaulAngus @GabrielBrascher @weizhouapache @RodrigoDLopez @DaanHoogland
we can quickly have either a +1 or -1 without further discussion and depending on that I will either close the pr or update the description of the pr
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902620414
> domain admin can only create service offering which are visible for accounts under his domain
> limited impaction (in domain admin's domain v.s. whole platform)
I'm afraid I can't see how your argument holds water.
you're saying it's okay to mess up in one domain as long as it isn't the entire platform?
How small are you assuming this domain to be? what if there are thousands of accounts under that domain? is it ok for the domain admin to mess up that domain, as long as they haven't messed up anywhere else?
Is there some mailing list thread that has a load of users complaining about this? It seems like this has become a point of principle, that a change with so little benefit is taking up so much oxygen.
I haven't given a -1, I just state that I think that it is wrong, due to _increased_ risk and negligible benefit.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-804999576
Thanks for fixing the conflicts @ravening.
## Here are my two cents:
From what I understood, the main discussion has been due to confusion around the scope and goals of this PR; I would like to bring some consensus and align the goals and expectations so we all _speak the same language_.
**Note:** this feedback is based on what I understood out of a quick check on all comments. Please let me know if I am missing some points here.
### Main conserns
If I understood it correctly, the main issue raised for this PR was regarding the fact that Domain admins **cannot** create public offerings.
I totally understand and agree with @PaulAngus' concerns; we need to keep the behavior of **Domain admins** creating only **non-public** offerings. If that was not the case then we would need to escalate to a deeper discussion to decide whether to change such behavior on the API or not. However, I think that this PR properly avoids the creation of public offerings by domain admins.
### Conclusion
I am still **+1** on having this merged, I don't see any harm in domain admins setting offerings to its domain, by default. When a ROOT admin creates an offering it is already set on its own domain. The code and automated/manual tests are aligned with the description.
The only point that may need some attention is regarding the documentation. According to the [API documentation](https://cloudstack.apache.org/api/apidocs-4.15/apis/createServiceOffering.html) a NULL _domainid_ results in a public offering, which is not the case of domain admins; on the other side, it is not a required parameter.
> domainid: the ID of the containing domain(s), null for public offerings`
#### Pros:
1. personally I think it enhances the user experience, adding flexibility and avoiding "unnecessary" error messages
2. I myself expected _domain id_ to not be required due to the API description
2. it keeps the offerings **NOT** public for domain-admins, regardless of the _domainid_
3. offerings created by domain admins are still under his own domain
4. domain admins can still create offerings for sub-domains by adding the sub-domain ID
#### Cons:
1. We might need to update the API description by stating the behavior when domain-admins keep it NULL
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-741765161
@rhtyd moved it to 4.16 and it needs a rebase, so let's discuss further after release, @weizhouapache.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-808239217
I'm still +1 on this but do see merit in the "public offering argument", We can make sure on public offering the current users domain is not set and an explicit domainid is required.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902654251
> > negligible benefit.
>
> I think the benefit is commercial, but can't be sure. and yes, oxygen is getting low. put it behind a setting that defaults to false please.
I think this pr has no risk, the new global setting looks too complicated and unnecessary in my opinion.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-806251108
<b>Trillian test result (tid-249)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36495 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4200-t249-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_migrate_VM_and_root_volume | `Error` | 84.55 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 57.32 | test_vm_life_cycle.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-712861361
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2224
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-903736477
So ... Oxygen is getting low indeed. Let's try to get some air.
I do understand that the benefits are not **_big_** with this PR and that there are raised concerns, which I respect; even though I do not completely agree with the raised risks nor the global settings approach.
What can we do in order to achieve a consensus?
@PaulAngus @DaanHoogland do you think that adding a global settings parameter would make this PR good to go? Would such global settings be needed on other API calls similar? For instance, the network case raised by @weizhouapache [here](https://github.com/apache/cloudstack/pull/4200#issuecomment-902567828).
I see the recent comment from @RodrigoDLopez. I do think that we might be missing some cases; I do appreciate any scenario example that would clarify such risks.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909103406
Sorry Rakesh, I'm -1 as it may introduce security/operational/access risks and issues, and points raised by others on this PR. I haven't tested or reviewed it though.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-677713456
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-659649127
@PaulAngus @ravening If I understood it correctly, this PR assumes that if the domain-admin does do not add a domain ID then he/she is creating an offering to its own domain ID.
With that in mind, I am **+1** with this PR proposal.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-665577184
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-677676769
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-724956794
> > > code lgtm, @ravening please make sure there are primate changes as well
>
> >
>
> > @DaanHoogland i think this is only backend change.
>
>
>
> @ravening, how about the changes in `ui/scripts/configuration.js`, then?
@DaanHoogland they are bug fix changes for displaying output in ui. I guess they're not needed for new ui. I will cross verify tomorrow
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909090346
@rhtyd @PaulAngus @GabrielBrascher @weizhouapache @RodrigoDLopez @DaanHoogland
we can quickly have either a +1 or -1 without further discussion and depending on that I will either close the pr or update the description of the pr
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-905701605
@PaulAngus Now I do understand your viewpoint. But I don't see the lack of attention as a risk. Using the same approach, I would say that we need to lock many parameters to ensure the offering will be created at the right domain and this is not the reality of ACS, we have some flexibility at some points. But "With great power comes great responsibility"
In my opinion, the proposal is too simple and will bring more flexibility to create offerings. Again, I can't see any damage caused by enabling subdomain admins to create offerings without pass the domain id, but they need to pay attention to what are they doing.
I agree with @GabrielBrascher
> I would be "-1" only in the case of a domain admin creating public offerings when the domain ID is NULL. However, this has been already checked and that's not the case here.
We already checked this point, and the created offering won't be a public one.
@ravening you do have my **+1** on this one. But for this, we need to update the API description
@PaulAngus, @DaanHoogland
Is there anything else that could be done by @ravening to proceed with this proposal?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902538321
> @weizhouapache clear except,
>
> > @rhtyd See my reply inline:
> > > @DaanHoogland I suppose either some arguments to answer the concerns - security or billing worries; or a global setting to toggle between the feature. For example, some test cases (either manually tested or a smoketests exists or is added):
>
> ...
>
> > > What if I'm domain admin of a sub-domain; can the offering be used by accounts of parent domain?
> >
> >
> > No. except the other account is ROOT admin.
>
> so if a ROOT admin account uses the offering, can the domain admin no longer delete it? or will it be copied/moved to the admins domain?
@DaanHoogland
cloudstack allows admins to delete service offering which are still used by vms.
the removal of service offering (no matter by admin or domain admin) will not cause issues I think.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902543977
What is the scope of this PR? the conversation seems to have moved in multiple directions. It seems like this should be separate threads, probably of separate PRs.
+ I still can't see why it's such a big deal for a domain admin to include the domain id of where they want an offering created.
If they're using the UI we can set the domain field to default to that domain, if they're using the API directly (ie via cmk) then it's a good validation that they're doing what they meant to, and if they're using some automation then the automation should deal with it.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909222161
@ravening
I'm +1 with this one. At this point, I cannot see any risk or damage caused by this PR. As I said, I don't see the lack of attention as a risk and it's not.
But I think the better solution is close this one. Since it won't go through.
Maybe on an close future you can reopen this discussion, when we have more cases where this flexibility will be appropriate to the context
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-659945306
> @PaulAngus @ravening If I understood it correctly, this PR assumes that if the domain-admin does do not add a domain ID then he/she is creating an offering to its own domain ID.
>
> With that in mind, I am **+1** with this PR proposal.
@GabrielBrascher yes. if the domain-admin, does not pass "domainid" parameter while creating an offering, then it will be created only for his domain, and it's not public.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-907033109
> @PaulAngus, @DaanHoogland
> Is there anything else that could be done by @ravening to proceed with this proposal?
@RodrigoDLopez , as you can see this PR already had my approval for a long time. I am just trying to address @PaulAngus' concern, which I think is not completely without merit. To me it doesn't warrant blocking the PR, though.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-903736477
So ... Oxygen is getting low indeed. Let's try to get some air.
I do understand that the benefits are not **"_big_"** with this PR and that there are raised concerns, which I respect; even though I do not completely agree with the raised risks nor the global settings approach.
What can we do in order to achieve a consensus?
@PaulAngus @DaanHoogland do you think that adding a global settings parameter would make this PR good to go? Would such global settings be needed on other API calls similar? For instance, the network case raised by @weizhouapache [here](https://github.com/apache/cloudstack/pull/4200#issuecomment-902567828).
I see the recent comment from @RodrigoDLopez. I do think that we might be missing some cases; I do appreciate any scenario example that would clarify such risks.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-905545602
> Hey, @PaulAngus I'm trying to see the risks you mention in every post on this conversation. But, at this point, I can see none. Maybe the others guys involved didn't catch that as well.
> So, to clarify to us all. could please explain those risks? I'm really trying to see the damage but looks like I'm totally blind
OK. So I've meant plenty of root admins who were not clear on how many aspects of how CloudStack works, Domain Admins are highly likely to have even less understanding than root admins.
As it stands when a domain admin adds an offering, they are required to specify the domain to which they are adding the offering. This helps ensure that the offering is added where it was intended.
There are a multitude of reasons and scenarios where a domain admin could forget/not realise/not know that the offering that they are adding would not only apply to the sub-domain that they are _thinking_ about at the time.
**Increasing** the risk of inadvertent mistakes for the sake of omitting domainid=<UUID> **if** the admin is using a CLI client, does not make sense to me. And to be honest, there are so many parameters in a service offering, removing the need to define _one_ is a drop in the ocean in terms of any reduction of effort.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-903595193
> > negligible benefit.
>
> I think the benefit is commercial, but can't be sure. and yes, oxygen is getting low. put it behind a setting that defaults to false please.
It would be very disappointing if what I see as unnecessarily increasing risk is being refuted for commercial reasons.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-726894671
> @ravening I am absolutely -1 on allowing admins to screw-up by not requiring the domain id, there is no need. Requiring it does not stop admins from doing anything
>
> isrecursive might be nice though.
@PaulAngus think about the way how virtual machine is deployed by domain admin. They dont pass domainid parameter while creating the VM. So according to you, we should disable creating vm without passing domainid as well right?
```
(test1) 🐱 > list accounts filter=rolename
{
"account": [
{
"rolename": "Domain Admin"
}
],
"count": 1
}
(test1) 🐱 >
(test1) 🐱 > deploy virtualmachine
💩 Missing required parameters: serviceofferingid, templateid, zoneid
(test1) 🐱 >
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-677676036
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] PaulAngus commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
PaulAngus commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-739813850
There is a world of difference between being able to create an offering and being able to create VM **on** that offering. You simply cannot compare them.
I'd like to see a feasible example of why this change is 'OK'
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-712788424
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-909222161
@ravening
I'm +1 with this one. At this point, I cannot see any risk or damage caused by this PR. As I said, I don't see the lack of attention as a risk and it's not.
But I think the better solution is close this one. Since it won't go through.
Maybe on an close future you can reopen this discussion, when we have more cases where this flexibility will be appropriate to the context
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902679167
I would be "-1" only in the case of a domain admin creating public offerings when the domain ID is NULL. However, this has been already checked and that's not the case here.
My main concern right now is with the API description that would need some updates in [createServiceOffering](https://cloudstack.apache.org/api/apidocs-4.15/apis/createServiceOffering.html). Right now it gives the idea of creating public offerings:
```
Parameter: domainid
Description: the ID of the containing domain(s), null for public offerings
Required: false
```
With that said, I do not see risks with this PR. Domains admins create offerings to:
**A.** their domain (not public)
**B.** their subdomains (it still not public)
The proposal would be to allow creating offerings on **A** by default. In the case of **B** then the domain admin would specify the domain ID.
It is not like the domain admin will create offerings outside its domain scope nor a public offering that other users can access.
In the worst-case, an ADMIN by mistake creates an offering to its domain, instead of to a specific subdomain. In my opinion, this does not bring great risks that would justify **-1**.
As @weizhouapache mentioned, shared network _acltype_ and _domain/account_ are not required. So far I have never seen any issue in a CloudStack zone due to such liberty given to Admins, for example, the one raised by @weizhouapache. On the other hand, I've seen numerous cases where Admins complain about too many restrictions.
I see that global settings might be an approach, but personally, I prefer to avoid global settings as much as possible.
We might go in a direction of multiple easter eggs hidden in the global settings where users will touch and understand just the 'tip of the iceberg'.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902567828
> What is the scope of this PR? the conversation seems to have moved in multiple directions. It seems like this should be separate threads, probably of separate PRs.
>
> * I still can't see why it's such a big deal for a domain admin to include the domain id of where they want an offering created.
> If they're using the UI we can set the domain field to default to that domain, if they're using the API directly (ie via cmk) then it's a good validation that they're doing what they meant to, and if they're using some automation then the automation should deal with it.
we could enfoce users to pass domainid and account when they deploy a instance. but we all agree it is unnecessary. users are smart and they know what they are doing.
there is another example, when create a shared network by root admin (networks are more important than service offerings, right ?), acltype and domain/account are optional not required. if root admin forgets to pass acltype, the created network will be visible by all users. is it more risky ?
if a change has no risk but make life a little bit easier, why not accept it ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-901690463
@DaanHoogland I suppose either some arguments to answer the concerns - security or billing worries; or a global setting to toggle between the feature. For example, some test cases (either manually tested or a smoketests exists or is added):
- Can I as a domain admin create an offering without domain ID, but that offering is used by other accounts outside of the domain? What if I'm domain admin of a sub-domain; can the offering be used by accounts of parent domain?
- What are the scope of the offering/settings that domain admin can add, for ex. can I specify storage tag etc?
cc @ravening
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-903731037
Hey, @PaulAngus I'm trying to see the risks you mention in every post on this conversation. But, at this point, I can see none. Maybe the others guys involved didn't catch that as well.
So, to clarify to us all. could please explain those risks? I'm really trying to see the damage but looks like I'm totally blind
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4200:
URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-901711561
> @DaanHoogland I suppose either some arguments to answer the concerns - security or billing worries; or a global setting to toggle between the feature. For example, some test cases (either manually tested or a smoketests exists or is added):
>
> * Can I as a domain admin create an offering without domain ID, but that offering is used by other accounts outside of the domain? What if I'm domain admin of a sub-domain; can the offering be used by accounts of parent domain?
> * What are the scope of the offering/settings that domain admin can add, for ex. can I specify storage tag etc?
>
> cc @ravening
@rhtyd @DaanHoogland @PaulAngus
cloudstack already allows domain admin to create service/disk offering since 2014
(see commit "CLOUDSTACK-7983: Create Disk/Service Offering for Domain Admin")
@shwstppr made some changes to support multiple domains/zones in 2019
(see commit "server: changes for domain, zone specified service offerings")
my opinion is, this is just an simple improvement of the feature.
(1) as root admin, "create offering without domainid" = "create offering with domainid=<ROOT domain>"
(2) as domain admin, "create offering without domainid" = "create offering with domainid=<domain id of account>"
that's it. I do not see any risk.
If you have concerns or objections on the feature, we can discuss in another thread.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org