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

chore: make const policy an extern #1587

Merged
merged 11 commits into from
Feb 3, 2025
Merged

Conversation

rishav-karanjit
Copy link
Member

@rishav-karanjit rishav-karanjit commented Jan 23, 2025

Issue #, if available:

Description of changes:

Dafny does not support transpiling a constant inside an extern for Go. So, this PR adds a {:extern} into the constant policy. This will need an update to the extern because dafny will no longer generate code for policy but we need to write it as it is an extern. This is not a breaking change because we are only writing code in extern instead of dafny generating the code and no customer facing changes are expected.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rishav-karanjit rishav-karanjit requested a review from a team as a code owner January 23, 2025 00:50
Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

@rishav-karanjit rishav-karanjit changed the title const in extern chore: add extern to constant inside extern Jan 27, 2025
@rishav-karanjit rishav-karanjit marked this pull request as draft January 27, 2025 21:36
@rishav-karanjit rishav-karanjit marked this pull request as ready for review January 27, 2025 21:44
@rishav-karanjit rishav-karanjit changed the title chore: add extern to constant inside extern chore: add extern to constant inside extern class Jan 27, 2025
@@ -8,7 +8,18 @@ namespace software.amazon.cryptography.dbencryptionsdk.dynamodb.itemencryptor.in

public partial class InternalLegacyOverride
{

public software.amazon.cryptography.dbencryptionsdk.dynamodb.internaldafny.types._ILegacyPolicy _policy
Copy link
Member Author

@rishav-karanjit rishav-karanjit Jan 27, 2025

Choose a reason for hiding this comment

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

I have tried following before reaching to this as solution:

  • Directly add a constant _policy in the class: This does not work because it should also be initialized with a constant in RHS and create_FORBID__LEGACY__ENCRYPT__FORBID__LEGACY__DECRYPT is not a constant.
  • Add a default constructor: We could have added a _policy and initialize it in a default constructor but this will not work because dafny transpiles a default constructor into class InternalLegacyOverride and this will cause conflict.

So, the best solution here is to always return FORBID__LEGACY__ENCRYPT__FORBID__LEGACY__DECRYPT from the getter. This is the only place, this variable is going to be used. However, technically this variable is never used because config.internalLegacyOverride.Some? is always false for .NET (and Rust) because they don't support legacy

@rishav-karanjit rishav-karanjit changed the title chore: add extern to constant inside extern class chore: make const policy a extern Jan 28, 2025
@rishav-karanjit rishav-karanjit changed the title chore: make const policy a extern chore: make const policy an extern Jan 28, 2025
@rishav-karanjit rishav-karanjit merged commit be3b96e into main Feb 3, 2025
39 checks passed
@rishav-karanjit rishav-karanjit deleted the rishav/constinExtern branch February 3, 2025 22:39
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