Refactor LogStorageFactory implementation to use Descriptor/Describable pattern
#132
Conversation
| <dependency> | ||
| <groupId>software.amazon.awssdk</groupId> | ||
| <artifactId>ssooidc</artifactId> | ||
| <version>2.31.33</version> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
I needed this to do basic testing directly against the AWS credentials available in my organization. It doesn't work for testing stuff related to the agent role assumption logic (seems like it should be possible with AWS_CHAINED_ROLE, but I couldn't quite get that working after spending a few minutes with it, but will take another look at it next week), and here I am not changing any of that logic anyway.
There was a problem hiding this comment.
Ok, I confirmed that this is required even after getting AWS_ROLE to work (problem was that I was trying to use the display name instead of the ARN for the role).
src/main/java/io/jenkins/plugins/pipeline_cloudwatch_logs/PipelineBridge.java
Show resolved
Hide resolved
| @@ -142,7 +142,7 @@ FormValidation validate(String logGroupName, String region, String credentialsId | |||
| // TODO should also check DescribeLogStreams, and perhaps even CreateLogStream and PutLogEvents, to ensure roles are correct | |||
| } catch (Exception x) { | |||
| String msg = processExceptionMessage(x); | |||
| return FormValidation.error(StringUtils.abbreviate(msg, 200)); | |||
| return FormValidation.error(abbreviate ? StringUtils.abbreviate(msg, 200) : msg); | |||
There was a problem hiding this comment.
I think this was just missed in #64. I needed the full message to debug some errors while I was configuring things for the tests.
| @@ -112,6 +112,26 @@ and as such requires several environment variables to be set: | |||
| * `AWS_REGION` should be set to, e.g., `us-east-1`. | |||
| * `CLOUDWATCH_LOG_GROUP_NAME` should be set to a test log group name, which you may need to precreate. | |||
|
|
|||
| You may configure the above environment variables in your `~/.m2/settings.xml` and they will be applied automatically when running `mvn test`: | |||
There was a problem hiding this comment.
Just making it a bit easier to run the tests, see also the changes in pom.xml.
| @@ -140,4 +162,31 @@ | |||
| </plugin> | |||
| </plugins> | |||
| </build> | |||
| <profiles> | |||
| <profile> | |||
| <id>cloudwatch-logs-settings</id> | |||
There was a problem hiding this comment.
Applying the API changes in jenkinsci/workflow-api-plugin#417 to this plugin. CC @jgreffe
For now I've only tried to most minimal changes that keeps everything fully backwards compatible. If we are willing to break backwards compatibility and require users to reconfigure things, we could move all of the configuration from
CloudWatchAwsGlobalConfigurationintoPipelineBridge. I am not sure if there is a good way to preserve compatibility otherwise, at least not anything that would be simpler than the current state in this PR.Testing done
I ran the integration tests against AWS manually by setting up the necessary AWS configuration and environment variables as described in the README.
Submitter checklist