Skip to content

/ wildfly-core

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

[WFCORE-3995] forbid invalid Sensitivity Constraint attribute read-wr… #3426

Merged
merged 3 commits into from Apr 17, 2019
Merged

[WFCORE-3995] forbid invalid Sensitivity Constraint attribute read-wr… #3426

merged 3 commits into from Apr 17, 2019

Conversation

@soul2zimate
Copy link
Contributor

soul2zimate commented Aug 9, 2018 •
edited

…ite combinations.

issues: https://issues.jboss.org/browse/WFCORE-3995 and https://issues.jboss.org/browse/WFCORE-4061
@bstansberry

This comment has been minimized.

Sign in to view
Copy link
Contributor

bstansberry commented Aug 23, 2018

There's a couple problems in this area:

  1. SensitivityClassificationWriteAttributeHandler does not have any rollback handling. That's not a problem in this PR; it's just this PR made me realize it. I suspect other handlers like this may have the same issue. I filed https://issues.jboss.org/browse/WFCORE-4061

  2. Cross-attribute validation like what's done here must be done in an added step, as that allows composite ops to work without undue concern about order of steps in the composite. Without that this might break existing scripts. (It would be ok to change the AbstractSensitivity and then add the validation step, counting on the rollback to correct the AbstractSensitivity if the validation fails. But WFCORE-4061 would have to be corrected for that to be a correct approach.)
@soul2zimate soul2zimate force-pushed the soul2zimate:WFCORE-3995-master branch from c558f0b to 4fdd3fd Aug 28, 2018
@soul2zimate

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

soul2zimate commented Aug 28, 2018

I have rebased to

  • add rollback handling first for WFCORE-4061
  • remove some redundant code because attribute name should have been validated before execution.
  • validate attributes for WFCORE-3995
@soul2zimate

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

soul2zimate commented Apr 8, 2019

Hi @bstansberry can this be merged ?
@soul2zimate

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

soul2zimate commented Apr 12, 2019

retest this please
@bstansberry
bstansberry requested changes Apr 14, 2019
View changes
Copy link
Contributor

bstansberry left a comment

Sorry I let this fall in a crack! Minor change needed. Thanks for adding the rollback handling and for the good test coverage.
...org/jboss/as/domain/management/access/ApplicationClassificationConfigResourceDefinition.java Outdated
@@ -121,17 +119,29 @@ public void execute(OperationContext context, ModelNode operation) throws Operat

@Override
public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
final String attribute = operation.require(NAME).asString();
PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));

This comment has been minimized.

Sign in to view
Copy link
@bstansberry

bstansberry Apr 14, 2019

Contributor

If you need the address of the current op, use 'context.getCurrentAddress()'.

I don't think you need it though; see my next comment.
...org/jboss/as/domain/management/access/ApplicationClassificationConfigResourceDefinition.java Outdated
PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));
ModelNode modelNode = context.readResourceFromRoot(address).getModel();
// record model values for rollback handler
ModelNode configuredApplication = modelNode.get(ModelDescriptionConstants.CONFIGURED_APPLICATION);

This comment has been minimized.

Sign in to view
Copy link
@bstansberry

bstansberry Apr 14, 2019

Contributor

These lines can come after L128 and the first line becomes

ModelNode modelNode = resource.getModel();

The Resource from context.readResourceForUpdate includes the data that you need to restore.
soul2zimate added 3 commits Aug 27, 2018
@soul2zimate
[WFCORE-4061] ApplicationTypeConfigWriteAttributeHandler and Sensitiv…
Unverified
The committer email address is not verified.
Learn about signing commits
9009338 
…ityClassificationWriteAttributeHandler rollback handling
@soul2zimate
[WFCORE-4061] remove redundant else clause because attribute name sho…
Unverified
The committer email address is not verified.
Learn about signing commits
14a2e5a 
…uld have been validated before execution.
@soul2zimate
[WFCORE-3995] Validate Sensitivity Classification attributes and tests.
Unverified
The committer email address is not verified.
Learn about signing commits
Loading status checks…
344c72d
@soul2zimate soul2zimate force-pushed the soul2zimate:WFCORE-3995-master branch from 4fdd3fd to 344c72d Apr 15, 2019
@wildfly-ci wildfly-ci added the deps-ok label Apr 15, 2019
@soul2zimate

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

soul2zimate commented Apr 15, 2019

Thanks, I have updated this one.
@bstansberry
bstansberry approved these changes Apr 15, 2019
View changes
@jmesnil jmesnil merged commit 8368040 into wildfly:master Apr 17, 2019
8 checks passed
8 checks passed
Dependency Tree (Pull Request) - merge TeamCity build finished
Details
Full integration - Linux Finished TeamCity Build WildFly Core / Pull Request / WildFly Core Full - Integration Linux - JDK 8 : Tests passed: 4855, ignored: 134
Details
Full integration - Windows Finished TeamCity Build WildFly Core / Pull Request / WildFly Core Full - Integration - Windows - JDK 8 : Tests passed: 4848, ignored: 139
Details
Linux - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Linux - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Linux - Security Manager - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Windows - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Windows - JDK 8 (Pull Request) - merge TeamCity build finished
Details
@soul2zimate soul2zimate deleted the soul2zimate:WFCORE-3995-master branch Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bstansberry bstansberry

Assignees
No one assigned
Labels
deps-ok ready-for-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@soul2zimate @bstansberry @jmesnil @wildfly-ci
You can’t perform that action at this time.