Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom command role ACL files on classpath in Static Role API Checker #354

Closed
wants to merge 1 commit into from
Closed

Conversation

ProjectMoon
Copy link

Commit Message

This commit has a small refactoring of cloud-plugin-acl-static-role-based
to allow it to read files on the classpath that might have a different name
than "commands.properties". It also allows more than one file to be read from.

Rationale: Third-party plugins may want to keep their API command access level
configuration separate from the main file so as to reduce configuration
maintenance work during packaging and deployments.

Testing Performed

Ran the simulator locally and connected to it with Cloudmonkey. Ran sync and then executed some API commands to verify that they are not blacklisted (i.e. not found because CS could not read the file on the classpath).

@asfbot
Copy link

asfbot commented Jun 3, 2015

cloudstack-pull-requests #301 FAILURE
Looks like there's a problem with this pull request

@ProjectMoon
Copy link
Author

Looks like the Oracle VM3 build failed in the pul request check. Is that a problem on the master branch currently?

@rohityadavcloud
Copy link
Member

LGTM, though we need to test it by producing packages. In theory, the /etc/cloudstack/management should be on classpath so this should work.

@ProjectMoon
Copy link
Author

Is there anything I can do to assist with the testing?

@rohityadavcloud
Copy link
Member

@ProjectMoon can you share any test results? Is the spring xml gets into the compiled jar from users' perspective, how may I configure a different properties file, any docs in this regard?

@ProjectMoon
Copy link
Author

This would be a step-by-step way of how I tested the change. We will use mycommands.properties as the example.

  1. Add the new properties file name to the list in the Spring XML and then compile CloudStack.
  2. Create mycommands.properties.in in client/tomcatconf. Build should move this to WEB-INF/classes after compilation as mycommands.properties.
  3. Move some commands from commands.properties.in to mycommands.properties.in
  4. Build and run client with Jetty server.
  5. Use cloudmonkey sync command to verify it's reading all 479 commands in from both files.

In a production environment the administrator would just drop a new properties file on the classpath somewhere, typically the classes directory of WEB-INF.

@rohityadavcloud
Copy link
Member

@ProjectMoon my concern here is -- you're still making changes in the XML to make it work during build time, is there a way to make it configurable like rabbitmq plugin so one can add the xml at some place in /etc/cloudstack/management to change/add file names in the XML. (pardon my ignorance, I'm not well versed with spring)

@ProjectMoon
Copy link
Author

Yes, that is correct it would need to be done during build time. I can add a way to make the list of files configurable. I think it would be best to make a properties file that has a key and a comma-separated list of files to find on the classpath.

I don't want to stick make a new properties file with just one line though. Are there any properties file that go into /etc/cloudstack/management where I could stick this key-value?

@rohityadavcloud
Copy link
Member

@ProjectMoon can you rebase and re-submit the PR, the travis tests seems to be failing.

@ProjectMoon
Copy link
Author

Pull request branch has been updated.

@asfbot
Copy link

asfbot commented Jun 18, 2015

cloudstack-pull-requests #489 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link

asfbot commented Jun 18, 2015

cloudstack-pull-requests #490 SUCCESS
This pull request looks good

…hecker.

This commit has a small refactoring of cloud-plugin-acl-static-role-based
to allow it to read files on the classpath that might have a different name
than "commands.properties". It also allows more than one file to be read from.

Rationale: Third-party plugins may want to keep their API command access level
configuration separate from the main file so as to reduce configuration
maintenance work during packaging and deployments.
@asfbot
Copy link

asfbot commented Jun 25, 2015

cloudstack-pull-requests #654 SUCCESS
This pull request looks good

@asfgit asfgit closed this in 93b201d Jul 1, 2015
@ProjectMoon ProjectMoon deleted the master-gq branch July 6, 2015 11:15
rohityadavcloud pushed a commit that referenced this pull request Jan 20, 2021
Fixes #354

Requires backend change #4041

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
RodrigoDLopez pushed a commit to RodrigoDLopez/cloudstack that referenced this pull request Aug 23, 2022
…orithm' into '4.16.0.0-scclouds'

Improve setting vm.allocation.algorithm description

Closes apache#354

See merge request scclouds/scclouds!221
@blueorangutan
Copy link

@weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants