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

Logic to perform deployment to Cloudhub V2 #59

Merged
merged 69 commits into from
Jan 17, 2024
Merged

Conversation

gteixeira1
Copy link
Contributor

@gteixeira1 gteixeira1 commented Dec 16, 2022

Resolves #46

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2022

Unit Test Results

313 tests  +39   313 ✔️ +39   2m 27s ⏱️ + 1m 23s
  37 suites +  6       0 💤 ±  0 
  37 files   +  6       0 ±  0 

Results for commit 98fc480. ± Comparison against base commit ad74440.

♻️ This comment has been updated with latest results.

@manikmagar
Copy link
Contributor

@gteixeira1 @adesjardin CloudHub 2.0 Deployment Model seems to have a lot of overlap with Runtime Fabric Deployment Model. Instead of making this new module as CH 2 specific, should we abstract this model so we could support RTF deployment with any minor extensions to this base mode?

import com.avioconsulting.mule.deployment.api.models.WorkerSpecRequest
import com.avioconsulting.mule.deployment.internal.models.CloudhubAppProperties
import groovy.json.JsonOutput
import groovy.transform.ToString

@ToString
class RuntimeFabricDeploymentRequest extends ExchangeAppDeploymentRequest {

final MAX_SIZE_APPLICATION_NAME = 42
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this only be on RTF deployments or also CH v1 deployments and standalone?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be on CH v1 too. For standalone, the app-limit may depend on the MULE_HOME path due to overall File PATH restrictions from OS. Having it configurable for standalone might help to be consistent in naming strategy.

* docs: Add README documentation about enabledFeatures

* docs: Fix documentation

* docs: Improved documentation for each flag of enabledFeatures
adesjardin
adesjardin previously approved these changes Jun 28, 2023
…es (#64)

* docs: Add README documentation about apiSpec section and its attributes

* docs: Fix designCenterBranchName attribute name in the examples
talyssoncastro and others added 12 commits June 28, 2023 13:27
…hen do not deploy API manager (#65)

* feat: Add method to retrieve existing API in the interface IApiManagerDeployer.groovy

* feat: Retrieve API id from API Manager and set into the application when do not deploy API manager

* feat: Add examples about API specification and API id retrieval

* test: Change unit tests to attend the new implementation of set API id without API manager deploy
* docs: Added sample custom policy configuration

* chore: deployment app name refactor

* chore: updates to make all tests pass

* chore: Use Java 11 on GitHub actions build

* chore: Use Java 17 on GitHub actions build

* fix: updating test projects to maven 3.5.4

* fix: updating test projects to maven 3.8.2

* fix: updates for java 8

* fix: Optional object needed additional .get()

* fix: updated tests to use the new applicationName object

* fix: updated tests to use the new applicationName object

* chore: Remove unnecessary code

---------

Co-authored-by: Kevin King <36522886+kkingavio@users.noreply.github.com>
Co-authored-by: Francisco Rodriguez Rojas <fr.rodriguez@pm.me>
Co-authored-by: Kevin King <kking@avioconsulting.com>
Co-authored-by: Talysson Castro <talyssoncastro@gmail.com>
* Resolve conflicts

* feat: Validate if mainRamlFile exists on separated method

* feat: Support API manager and Policies deploy when using CH 2.0 and RTF deployment

* refactor: Use base class AppDeploymentRequest.groovy and create getUnsupportedFeatures

* feat: Return null for RAML files for ExchangeAppDeploymentRequest and derived classes

* feat: Move DesignCenter as unsupported to the proper class - ExchangeAppDeploymentRequest

* feat: Improve parameter name

* docs: Improve examples

* docs: Add example of CH 2.0 with API Manager and Policy sync

* feat: Encapsulate logic of testing null on RAML files

* feat: Add autoDiscoveryId property into deployment properties for RTF and CH 2.0

* Use ApplicationName in tests

* Requires appName for RTF and CH 2.0 applications

* Fix CloudHub integration test

* test: Unit test about validating empty name for RTF and CH 2.0 apps
* chore: Remove cloudHubAppPrefix from docs and unit tests
* test: Create suites for integration and unit tests to run separated
* build: Add condition to run only unit test when branch is not master
Comment on lines 4 to 5
applicationName
applicationName {
Copy link
Contributor

@manikmagar manikmagar Jan 4, 2024

Choose a reason for hiding this comment

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

Looks like left a duplicate by mistake.

}

private void runGuards(){
assert (baseAppName != null && !baseAppName.isBlank() && !baseAppName.contains(" ")) : "you should specify an non-empty baseAppName. It shouldn't contain spaces as well"
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Are special characters other than spaces allowed as app name? Eg. some#app*&$name. Current validation accepts that name. Maybe just restricting it to Alphanumeric and Dash is better?
  2. Error message formatting, could be a simple message Application name cannot be empty and cannot contain spaces. If more characters are not allowed then probably we can just say that "Name must be alphanumeric and can include dash."

Comment on lines 216 to 219
#boolParam('usePrefix')
#boolParam('useSuffix')
#stringParam('prefix')
#stringParam('suffix')
Copy link
Contributor

@manikmagar manikmagar Jan 4, 2024

Choose a reason for hiding this comment

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

Curious to know if we need pairs of properties eg usePrefix and prefix? Can the existence of valid prefix or suffix strings not be used to decide and reduce the configuration elements? Changing this later would mean breaking change, so asking before 2.0 is released.

Comment on lines 119 to 120
<version>${junit.platform.version}</version>
<scope>test</scope>
Copy link
Contributor

@manikmagar manikmagar Jan 4, 2024

Choose a reason for hiding this comment

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

Should we move this dependencyManagement in parent pom? Could be done later too.

talyssoncastro and others added 6 commits January 5, 2024 15:02
… parent POM and use them in children's projects (#88)
* feat: Change valid characters logic in the application name

* test: Fix and add unit tests for application name feature

* Improve regex to validate application name

Co-authored-by: Manik Magar <manik.magar@gmail.com>

* feat: Use the new validateName method to validate application name

* test: Fix unit tests for the new message about invalid application name

---------

Co-authored-by: Manik Magar <manik.magar@gmail.com>
* feat: Remove usePrefix and useSuffix attributes

* docs: Remove usePrefix and useSuffix from examples

* test: Fix unit tests that use usePrefix and useSuffix properties

* test: Fix unit tests that use usePrefix and useSuffix properties

* test: Keep unit tests with original behaviour for usePrefix and useSuffix

* test: Keep unit tests with original behaviour for usePrefix and useSuffix
* docs: Add documentation for usages of ApplicationName feature

* docs: Remove usePrefix and useSuffix from README documentation
@talyssoncastro talyssoncastro merged commit eda9e70 into master Jan 17, 2024
5 checks passed
@talyssoncastro talyssoncastro deleted the feat/cloudhub2 branch January 17, 2024 21:03
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.

CloudHub v2 Depoyment Support
5 participants