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

DATAGO-64296: initial commit for terraform plugin #132

Merged
merged 22 commits into from
Nov 24, 2023

Conversation

gregmeldrum
Copy link
Collaborator

@gregmeldrum gregmeldrum commented Nov 23, 2023

What is the purpose of this change?

This PR adds the new terraform plugin to the EMA. The plugin is added as a new module.
Additionally, a temporary REST API has been added to allow testing
The REST API follows the newly approved command model

How was this change implemented?

The terraformClient has been copied and modified from https://github.com/microsoft/terraform-spring-boot
The RESP API is added to the application module since this module takes care of the northbound interface
The terraform specific code is added to the terraform-plugin module.

Some work has been deferred to later stories such as:

  • logging support (some logging support is included but it will be enhanced in the future)
  • building the docker image containing the terraform executable and solace provider
  • adding a messaging API for Solace Cloud Event Portal
  • functional testing for all supported usecases (in this PR only simple adding/deleting of queues is tested)
  • negative functional testing
  • non-functional testing (performance and load)

How was this change tested?

  • Simple functional tests were run using the REST API via POSTMAN commands.
  • Integration tests were run using the TerraformClientRealTests test class in the terraform-plugin
  • Unit tests were added to test the basic terraform command handling using a mocked terraform client.

Is there anything the reviewers should focus on/be aware of?

Just re-iterating, some of the code was taken from open-source (MIT license) at https://github.com/microsoft/terraform-spring-boot
The following classes were copied and modified:
ProcessLauncher
TerraformClient
Our lawyer gave the thumbs up to use this code as long as we include a link to it's origin and a copy of the license under which it was released.

@@ -0,0 +1,28 @@
package com.solace.maas.ep.event.management.agent.command.rest;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code will be removed once the messaging I/F is added.

sendLogsAsync(event,
event.getMDCPropertyMap().get(RouteConstants.SCAN_ID),
event.getMDCPropertyMap().get(RouteConstants.TRACE_ID),
event.getMDCPropertyMap().get(RouteConstants.ACTOR_ID),
event.getMDCPropertyMap().get(RouteConstants.SCAN_TYPE),
event.getMDCPropertyMap().get(RouteConstants.SCHEDULE_ID),
event.getMDCPropertyMap().get(RouteConstants.MESSAGING_SERVICE_ID));
} else if (StringUtils.isNotEmpty(event.getMDCPropertyMap().get(RouteConstants.COMMAND_CORRELATION_ID))) {
log.trace("This is a placeholder for DATAGO-64298");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the hook where we will add the terraform logs in the future.

@@ -3,7 +3,9 @@
public enum MOPProtocol {
scanData(2850),
scanDataControl(2851),
EMAHeartbeat(2852);
EMAHeartbeat(2852),
commandProtocol(2853);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was added for future considerations when we add the messaging API

@@ -237,22 +239,6 @@
</resources>
</configuration>
</execution>
<execution>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused execution

@@ -0,0 +1,149 @@
package com.solace.maas.ep.event.management.agent.plugin.terraform.client;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file comes from open-source with light modifications.

@@ -0,0 +1,197 @@
package com.solace.maas.ep.event.management.agent.plugin.terraform.client;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file comes from open source with moderate modifications.

@Service
@Data
public class TerraformProperties {
@Value("${plugins.terraform.workingDirectoryRoot:/${HOME}/tfconfig}")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an unorthodox way of setting properties. @value is not the recommended way to populate config. In this case I wanted a convenient way to set a default value for this value without requiring a change to the application.yml file. Alternatively, we could add a backend story to EP to create a new plugins.terraform.workingDirectoryRoot property to the generated application.ylm file.

Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 4.0% 4.0% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

String traceId = MDC.get("traceId");
String spanId = MDC.get("spanId");

log.debug("Executing command {} for ms {} correlationId {} context {}", command.getCommand(), request.getServiceId(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

please change ms to some generic name like service since it won't work for schemaRegsitry configPush scenarios

@gregmeldrum gregmeldrum merged commit d19dfea into main Nov 24, 2023
2 of 3 checks passed
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.

2 participants