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

refactor(sdk): remove deprecated EncryptWithAES code #1235

Merged

Conversation

marcpfuller
Copy link
Contributor

@marcpfuller marcpfuller commented Dec 1, 2022

BRAKING CHANGE: remove deprecated EncryptionWithAES code, use AESProtection

CLoses: #1227

Signed-off-by: Marc-Philippe Fuller marc-philippe.fuller@intel.com

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/app-functions-sdk-go/blob/main/.github/CONTRIBUTING.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?) N/A
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)
    fix: remove references to deprecated code for EncryptWithAES edgex-docs#919

Testing Instructions

make test

New Dependency Instructions (If applicable)

N/A

internal/app/configurable.go Outdated Show resolved Hide resolved
@marcpfuller marcpfuller force-pushed the remove_deprecated_encryptionAES branch 2 times, most recently from b3cad37 to 78f5b74 Compare December 5, 2022 21:39
bnevis-i
bnevis-i previously approved these changes Dec 13, 2022
@lenny-goodell lenny-goodell changed the title refactor(sdk): remove deprecated EncryptWithAES code refactor(sdk)!: remove deprecated EncryptWithAES code Jan 3, 2023
@lenny-goodell
Copy link
Member

@marcpfuller , this PR needs to be rebased and fix conflicts.
Also need corresponding Docs PR.

BRAKING CHANGE: remove deprecated EncryptionWithAES code, use AESProtection

CLoses: edgexfoundry#1227

Signed-off-by: Marc-Philippe Fuller <marc-philippe.fuller@intel.com>
@bnevis-i
Copy link
Collaborator

bnevis-i commented Jan 3, 2023

@marcpfuller Failed test case

023-01-03T23:00:13.730Z] --- FAIL: TestEncrypt (0.00s)

[2023-01-03T23:00:13.730Z]     --- FAIL: TestEncrypt/Good_-_Key_&_vector_ (0.00s)

[2023-01-03T23:00:13.730Z]         configurable_test.go:422: 

[2023-01-03T23:00:13.730Z]             	Error Trace:	/w/workspace/dry_app-functions-sdk-go_PR-1235/internal/app/configurable_test.go:422

[2023-01-03T23:00:13.730Z]             	Error:      	Not equal: 

[2023-01-03T23:00:13.730Z]             	            	expected: false

[2023-01-03T23:00:13.730Z]             	            	actual  : true

[2023-01-03T23:00:13.730Z]             	Test:       	TestEncrypt/Good_-_Key_&_vector_

[2023-01-03T23:00:13.730Z]     --- FAIL: TestEncrypt/Good_-_Secrets_&_vector (0.00s)

[2023-01-03T23:00:13.730Z]         configurable_test.go:422: 

[2023-01-03T23:00:13.730Z]             	Error Trace:	/w/workspace/dry_app-functions-sdk-go_PR-1235/internal/app/configurable_test.go:422

[2023-01-03T23:00:13.730Z]             	Error:      	Not equal: 

[2023-01-03T23:00:13.730Z]             	            	expected: false

[2023-01-03T23:00:13.730Z]             	            	actual  : true

[2023-01-03T23:00:13.730Z]             	Test:       	TestEncrypt/Good_-_Secrets_&_vector

@marcpfuller
Copy link
Contributor Author

@bnevis-i, I resolved the failed testcase.

@marcpfuller marcpfuller requested review from bnevis-i and removed request for lenny-goodell January 4, 2023 00:59
@marcpfuller marcpfuller changed the title refactor(sdk)!: remove deprecated EncryptWithAES code refactor(sdk): remove deprecated EncryptWithAES code Jan 4, 2023
bnevis-i
bnevis-i previously approved these changes Jan 4, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1235 (8d39707) into main (a81394e) will decrease coverage by 0.29%.
The diff coverage is n/a.

❗ Current head 8d39707 differs from pull request most recent head fa918c9. Consider uploading reports for the commit fa918c9 to get more accurate results

@@            Coverage Diff             @@
##             main    #1235      +/-   ##
==========================================
- Coverage   66.48%   66.19%   -0.30%     
==========================================
  Files          39       38       -1     
  Lines        3297     3227      -70     
==========================================
- Hits         2192     2136      -56     
+ Misses        961      951      -10     
+ Partials      144      140       -4     
Impacted Files Coverage Δ
internal/app/configurable.go 62.77% <ø> (-1.51%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

@marcpfuller there is also a new binary file (internal/app/__debug_bin) that should not be added

@@ -378,21 +378,6 @@ func (app *Configurable) Encrypt(parameters map[string]string) interfaces.AppFun
}

switch strings.ToLower(algorithm) {
case EncryptAES:
Copy link
Member

Choose a reason for hiding this comment

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

Also need to remove the EncryptAES constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the constant, and all its references.

Signed-off-by: Marc-Philippe Fuller <marc-philippe.fuller@intel.com>
@marcpfuller marcpfuller force-pushed the remove_deprecated_encryptionAES branch from fa918c9 to b07c832 Compare January 4, 2023 18:25
@marcpfuller marcpfuller requested review from lenny-goodell and bnevis-i and removed request for lenny-goodell and bnevis-i January 4, 2023 18:26
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@lenny-goodell lenny-goodell merged commit 19e77ea into edgexfoundry:main Jan 4, 2023
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.

General cleanup for deprecated trigger/pipelines APIs
4 participants