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

[WFLY-18503] security-domain-to-domain Quickstart Common Enhancements… #749

Conversation

PrarthonaPaul
Copy link
Contributor

… CY2023Q3
Issue: https://issues.redhat.com/browse/WFLY-18503
This QS is open shift incompatible

@openshift-ci
Copy link

openshift-ci bot commented Oct 16, 2023

Hi @PrarthonaPaul. Thanks for your PR.

I'm waiting for a wildfly member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@PrarthonaPaul PrarthonaPaul force-pushed the security-domain-to-domain-enhancements branch 3 times, most recently from 6509aab to b7108c5 Compare October 20, 2023 15:52
@PrarthonaPaul PrarthonaPaul force-pushed the security-domain-to-domain-enhancements branch from b7108c5 to 577e325 Compare October 30, 2023 14:54
@PrarthonaPaul PrarthonaPaul force-pushed the security-domain-to-domain-enhancements branch from 577e325 to 0405a6a Compare November 22, 2023 16:54
@PrarthonaPaul
Copy link
Contributor Author

Hi @emmartins
This should be good to go as well. Just waiting for the CI to be completed.

</feature-packs>
<layers>
<!-- layers may be used to customize the server to provision -->
<layer>cloud-server</layer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the ejb layer needed too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 it's strange to me that the tests don't fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The profiles are in wrong pom.xml, this a multi module maven project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of fact this one is different than everything else so far, the deployment dir is the "ear" module. I wonder if we need to drop that (and instead pack the ejb in the war) to use server provisioning, will try to find time to investigate this later today.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PrarthonaPaul due to ^^ I may end up sending you a PR to fix everything, will let you know after investigation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!
Thanks, everyone for noticing it!

pull_request:
types: [opened, synchronize, reopened, ready_for_review]
paths:
- security-domain-to-domain/**'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- 'security-domain-to-domain/**'

//*************************************************
// Product Release content only
//*************************************************
ifdef::ProductRelease[]
:server_provisioning_server_host: https://localhost:8443
include::../shared-doc/build-and-run-the-quickstart-with-provisioned-server.adoc[leveloffset=+1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already included, please remove this line

<plugin>
<groupId>org.wildfly.plugins</groupId>
<artifactId>wildfly-maven-plugin</artifactId>
<configuration>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packaging script is missing

//*************************************************
// Product Release content only
//*************************************************
ifdef::ProductRelease[]
:server_provisioning_server_host: https://localhost:8443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this quickstart uses standard server host for test, please remove

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add attribute :deploymentDir: web on line 15.

@emmartins
Copy link
Contributor

WFLY 31.0.0.Beta1 has been released, please rebase this PR with upstream/main branch, and update:

  • All pom.xml to 31.0.0.Final-SNAPSHOT
  • All pom.xml to 6
  • Property named version.server to 31.0.0.Beta1
  • Property named version.plugin.wildfly to 4.2.1.Final

@PrarthonaPaul PrarthonaPaul force-pushed the security-domain-to-domain-enhancements branch from 0405a6a to 451f68e Compare December 21, 2023 14:41
@PrarthonaPaul PrarthonaPaul force-pushed the security-domain-to-domain-enhancements branch from 451f68e to faf7f9d Compare December 21, 2023 14:52
@PrarthonaPaul
Copy link
Contributor Author

Hi @emmartins
I have made the version updates to the pom.xml file and addressed the comments above.
This QS should be good to go! Thanks

@emmartins
Copy link
Contributor

Superseded by #887

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