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

Added first pass at ClamAv resources #412

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

csharpfritz
Copy link

**Closes #102 **

Adds a new ClamAv anti-malware resource that can be used to scan uploaded files

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

@csharpfritz
Copy link
Author

Rebased and cleaned up a merge conflict in SLN file

Copy link
Member

@Alirexaa Alirexaa left a comment

Choose a reason for hiding this comment

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

@csharpfritz, Thanks for the contribution.

This PR is in the right way, but some following improvements need to be resolved:

Is there any way to protect ClamAv with a user and password or API key? If yes, we should cover that.

Is there any health API for ClamAv? I see some unfinished work here. If not, please remove the ConnectionStringAvailableEvent event subscription part.

Is there any .NET client for ClamAv?

We need some tests for the hosting integration, client integration, and an example for usage. You can get help by looking at Meilisearch or Ollma integration tests.

/// <summary>clamav/clamav</summary>
public const string Image = "clamav/clamav";
/// <summary>latest</summary>
public const string Tag = "latest";
Copy link
Member

Choose a reason for hiding this comment

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

We have policy to not use latest tag beacase it's not reliable. Could you use tag in major.minor or major.minor.patch format here?

Copy link
Author

Choose a reason for hiding this comment

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

I'll update to 1.4.1

@csharpfritz
Copy link
Author

csharpfritz commented Jan 22, 2025

@csharpfritz, Thanks for the contribution.

This PR is in the right way, but some following improvements need to be resolved:

Is there any way to protect ClamAv with a user and password or API key? If yes, we should cover that.

I have not seen a way to do this.

Is there any health API for ClamAv? I see some unfinished work here. If not, please remove the ConnectionStringAvailableEvent event subscription part.

I can look further into this, its not completely clear from their docs at https://github.com/openbridge/clamav

Is there any .NET client for ClamAv?

There is, nClam is available and I had started working on a wrapper for that to include OTel interactions

We need some tests for the hosting integration, client integration, and an example for usage. You can get help by looking at Meilisearch or Ollma integration tests.

/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<ClamAvResource> AddClamAv(
this IDistributedApplicationBuilder builder,
string name,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
string name,
[ResourceName] string name,

Ensures that the analyzer is run

Copy link
Member

Choose a reason for hiding this comment

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

Can we moved this to the Unshipped file, since we haven't shipped the integration yet.

@github-actions github-actions bot added the Stale label Jan 28, 2025
@aaronpowell aaronpowell added awaiting response Waiting for the author of the issue to provide more information or answer a question and removed Stale labels Jan 29, 2025
@github-actions github-actions bot added the Stale label Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Waiting for the author of the issue to provide more information or answer a question Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration proposal - ClamAv antivirus
3 participants