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

Move truststore creator to server package for cloud builds #21279

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

todvora
Copy link
Contributor

@todvora todvora commented Jan 7, 2025

/nocl internal refactoring

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@todvora todvora requested review from moesterheld and bernd January 7, 2025 10:23
Copy link
Contributor

@moesterheld moesterheld left a comment

Choose a reason for hiding this comment

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

Haven't run the cloud build locally (Bernd will), but the code changes lgtm.

Copy link
Member

@bernd bernd left a comment

Choose a reason for hiding this comment

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

The Cloud build now fails with the following error:

[ERROR] /home/bernd/graylog/cloud-builds/modules/graylog2-server/full-backend-tests/src/test/java/org/graylog/datanode/ClientCertResourceIT.java:[31,36] error: cannot find symbol
  symbol:   class TruststoreCreator
  location: package org.graylog.security.certutil
[ERROR] /home/bernd/graylog/cloud-builds/modules/graylog2-server/full-backend-tests/src/test/java/org/graylog/datanode/ClientCertResourceIT.java:[134,15] error: cannot find symbol
  symbol:   variable TruststoreCreator
  location: class ClientCertResourceIT
[INFO] 2 errors 

@bernd
Copy link
Member

bernd commented Jan 7, 2025

The Cloud build now fails with the following error:

[ERROR] /home/bernd/graylog/cloud-builds/modules/graylog2-server/full-backend-tests/src/test/java/org/graylog/datanode/ClientCertResourceIT.java:[31,36] error: cannot find symbol
  symbol:   class TruststoreCreator
  location: package org.graylog.security.certutil
[ERROR] /home/bernd/graylog/cloud-builds/modules/graylog2-server/full-backend-tests/src/test/java/org/graylog/datanode/ClientCertResourceIT.java:[134,15] error: cannot find symbol
  symbol:   variable TruststoreCreator
  location: class ClientCertResourceIT
[INFO] 2 errors 

The TruststoreCreator still lives in the data-node/ Maven module. Which we don't build.

@moesterheld
Copy link
Contributor

I think we shouldn't create packages other than org.graylog.datanode in the datanode module. That makes it easier to check and that's why I didn't see it right away.

bernd added a commit that referenced this pull request Jan 7, 2025
We have a "skip.datanode" build option that fails when using Data Node
related packages outside of the data-node Maven module.

This adds the restrict-imports-enforcer-rule enforcer plugin to get
access to the "RestrictImports" rules.

Refs #21279
@todvora
Copy link
Contributor Author

todvora commented Jan 7, 2025

Sorry for the confusion, moving class between packages in idea did something that I haven't expected and created new package in the same project, instead of moving to an existing package in the server project. Fixed now.

Copy link
Member

@bernd bernd left a comment

Choose a reason for hiding this comment

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

The Cloud build works again. Thanks!

@moesterheld moesterheld merged commit 80ba832 into master Jan 7, 2025
6 checks passed
@moesterheld moesterheld deleted the fix/move-truststore-creator-to-server branch January 7, 2025 16:52
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.

3 participants