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

Scan uploaded attachments #86

Merged
merged 27 commits into from
Jun 6, 2024
Merged

Scan uploaded attachments #86

merged 27 commits into from
Jun 6, 2024

Conversation

Ceredron
Copy link
Collaborator

@Ceredron Ceredron commented May 31, 2024

Description

Scan uploaded attachment.

Differs from Broker as it will not need to support >2GB files so we can use a shared storage account with the same Microsoft Defender configuration for all customers.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@Ceredron Ceredron changed the title Scan uploaded attachments WIP: Scan uploaded attachments May 31, 2024
…rred because more than one subscription was registered when testing last time.
@Ceredron Ceredron changed the title WIP: Scan uploaded attachments Scan uploaded attachments Jun 6, 2024
@Andreass2
Copy link
Collaborator

Code looks good.

But I am thinking we will get the same idempotency problem as we had i broker.
Implementing a idempotencyRepository can be done in a seperate issue though

.azure/infrastructure/params.bicepparam Outdated Show resolved Hide resolved
@Ceredron
Copy link
Collaborator Author

Ceredron commented Jun 6, 2024

Code looks good.

But I am thinking we will get the same idempotency problem as we had i broker. Implementing a idempotencyRepository can be done in a seperate issue though

I have tested it in Azure, and not been able to replicate it. I wonder if the original issue occurred because we had more than one malware scan subscription.

@Andreass2
Copy link
Collaborator

Code looks good.
But I am thinking we will get the same idempotency problem as we had i broker. Implementing a idempotencyRepository can be done in a seperate issue though

I have tested it in Azure, and not been able to replicate it. I wonder if the original issue occurred because we had more than one malware scan subscription.

It actually seems Azure Event Grid no longer sends multiple events(they did before). So as long as we can guarantee that we returns a 200 OK in under 30 seconds it should not be a problem

@Ceredron Ceredron merged commit 69cc9fb into main Jun 6, 2024
4 checks passed
@Ceredron Ceredron deleted the feat/virus-scan branch June 6, 2024 13:21
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