Skip to content

CPLYTM-550 - Clean-up openscap-plugin configuration file#52

Merged
marcusburghardt merged 2 commits into
complytime:mainfrom
marcusburghardt:clean-plugin-config
Feb 20, 2025
Merged

CPLYTM-550 - Clean-up openscap-plugin configuration file#52
marcusburghardt merged 2 commits into
complytime:mainfrom
marcusburghardt:clean-plugin-config

Conversation

@marcusburghardt
Copy link
Copy Markdown
Member

Summary

  • Server struct was only used in a very initial stage. Now the plugin is managed by ComplyTime.
  • Plugin directory doesn't need to be changed through a configuration file.
    • If there is any valid case for this in the future, we can introduce options on demand. For now, it is removed in favor of simplicity.

Related Issues

  • CPLYTM-550

Review Hints

No impact is expected on how the plugin and commands behave, so unit tests should be enough.

@marcusburghardt marcusburghardt requested review from a team, gvauter and jpower432 February 19, 2025 12:11
Copy link
Copy Markdown
Member

@jpower432 jpower432 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@gvauter gvauter left a comment

Choose a reason for hiding this comment

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

One small nit, but lgtm.

expectedPolicyPath := filepath.Join(tempDir, "workspace", "plugins", "policy", "policy.yaml")
expectedResultsPath := filepath.Join(tempDir, "workspace", "plugins", "results", "results.xml")
expectedARFPath := filepath.Join(tempDir, "workspace", "plugins", "results", "arf.xml")
expectedPolicyPath := filepath.Join(tempDir, "workspace", "openscap", "policy", "policy.yaml")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tiny nit pick - perhaps use the PluginDir constant instead of "openscap" here? Just in case it were to ever change there would be less places to refactor.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point @gvauter . I am preparing another PR on top of it and will already include this improvement. Thanks.

It was only used in a very initial stage. Now the plugin is managed by
ComplyTime.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
The plugin dir doesn't need to be changed. If there is any valid case for
this in the future, we can introduce options on demand. For now, it is
removed in favor of simplicity.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt marcusburghardt merged commit 9a24c54 into complytime:main Feb 20, 2025
@marcusburghardt marcusburghardt deleted the clean-plugin-config branch February 20, 2025 07:40
hbraswelrh added a commit to hbraswelrh/complyctl that referenced this pull request Jun 4, 2026
Replace the dummy ampel complypack payload in the mock OCI registry
with valid granular policy JSON that the ampel provider's
LoadGranularPolicies() accepts. The dummy content lacked the required
'id' field, causing cross-repo integration test failures when the
ampel provider consumes ComplypackContentPath (complytime-providers
PR complytime#52).

Changes:
- Add testdata/ampel-complypack/block-force-push.json with valid
  AmpelPolicy content (copied from cross-repo test fixture)
- Add //go:embed directive for ampel complypack testdata
- Update seedDefaults() to use buildTarGzFromFS instead of
  buildDummyTarGz for the complypacks/ampel-bp artifact
- Add TestBuildTarGzFromFS_AmpelFS verifying archive structure
  and JSON content validity
- Extend TestSeedDefaults_AllReposSeeded with ampel complypack
  content blob verification (manifest -> layer -> gzip -> tar ->
  JSON -> id field)

Follows the OPA complypack pattern established in commit 74fbae8.

Ref: complytime/complytime-providers#52
hbraswelrh added a commit to hbraswelrh/complyctl that referenced this pull request Jun 5, 2026
Replace the dummy ampel complypack payload in the mock OCI registry
with valid granular policy JSON that the ampel provider's
LoadGranularPolicies() accepts. The dummy content lacked the required
'id' field, causing cross-repo integration test failures when the
ampel provider consumes ComplypackContentPath (complytime-providers
PR complytime#52).

Changes:
- Add testdata/ampel-complypack/block-force-push.json with valid
  AmpelPolicy content (copied from cross-repo test fixture)
- Add //go:embed directive for ampel complypack testdata
- Update seedDefaults() to use buildTarGzFromFS instead of
  buildDummyTarGz for the complypacks/ampel-bp artifact
- Add TestBuildTarGzFromFS_AmpelFS verifying archive structure
  and JSON content validity
- Extend TestSeedDefaults_AllReposSeeded with ampel complypack
  content blob verification (manifest -> layer -> gzip -> tar ->
  JSON -> id field)

Follows the OPA complypack pattern established in commit 74fbae8.

Ref: complytime/complytime-providers#52
hbraswelrh added a commit to hbraswelrh/complyctl that referenced this pull request Jun 5, 2026
Replace the dummy ampel complypack payload in the mock OCI registry
with valid granular policy JSON that the ampel provider's
LoadGranularPolicies() accepts. The dummy content lacked the required
'id' field, causing cross-repo integration test failures when the
ampel provider consumes ComplypackContentPath (complytime-providers
PR complytime#52).

Changes:
- Add testdata/ampel-complypack/block-force-push.json with valid
  AmpelPolicy content (copied from cross-repo test fixture)
- Add //go:embed directive for ampel complypack testdata
- Update seedDefaults() to use buildTarGzFromFS instead of
  buildDummyTarGz for the complypacks/ampel-bp artifact
- Add TestBuildTarGzFromFS_AmpelFS verifying archive structure
  and JSON content validity
- Extend TestSeedDefaults_AllReposSeeded with ampel complypack
  content blob verification (manifest -> layer -> gzip -> tar ->
  JSON -> id field)

Follows the OPA complypack pattern established in commit 74fbae8.

Ref: complytime/complytime-providers#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