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
[2.150.0][deliver] better error when uses non exempt encryption not set #16646
[2.150.0][deliver] better error when uses non exempt encryption not set #16646
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, clear! 🚀
@@ -63,6 +63,17 @@ def update_export_compliance(options, app, build) | |||
|
|||
UI.verbose("Updating build for export compliance status of '#{uses_encryption}'") | |||
if build.uses_non_exempt_encryption.nil? | |||
if uses_encryption.to_s.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdholtz in the issue #16645 the export_compliance_uses_encryption
was set in accordance with http://docs.fastlane.tools/actions/deliver/#compliance-and-idfa-settings, but still got the error. It's as if it never got into this if.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added more details to the issue #16645 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is still not in prod, unless you have checked out the branch, but let's focus on reviewing the PR, we can discuss about issues in their places 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@minuscorp Sure, this PR looks good on its own, but it looks that it doesn't address the issue which motivated it. And yes, I'm testing with the 2.150.0.rc1 version :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you find where the submission_information[:export_compliance_uses_encryption]
is being set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that comes from the deliver config https://github.com/artsy/eigen/blob/faa02e2746194d8d7c11899474de9c517435eca4/fastlane/Fastfile#L131-L149
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant, internally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I get it, no I didn't find that...
].join("\n") | ||
UI.user_error!(message) | ||
end | ||
|
||
build = build.update(attributes: { | ||
usesNonExemptEncryption: uses_encryption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdholtz should we use export_compliance_is_exempt
instead of export_compliance_uses_encryption
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use snake_case representations of the keys from the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@minuscorp of course, but that was not my question :) If you look at the line 62, uses_encryption
is extracted from submission_information[:export_compliance_uses_encryption]
. and later used to set usesNonExemptEncryption
.
It looks to me that we should use submission_information[:export_compliance_is_exempt]
to set usesNonExemptEncryption
. Actually the negation of that :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjovkovic Thanks for this review! This is totally the case. You are right 😊 I got another PR that I was doing something different and noticed this and fixed it in there 🤷♂️ I'm going to merge his but I will have this fixed in in the 2.150.0
branch shortly. Sorry about that! ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually... I'm just gunna push it to this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjovkovic The different between
export_compliance_uses_encryption
andexport_compliance_is_exempt
is really confusing 🤔 I thinkexport_compliance_uses_encryption
is the correct one 😬 But this (along with other things) are named differently now that make it confusing...
I'm trying to compare those fields to the ones on Appstoreconnect when you need to provide compliance. Here are the questions. Depending on the answers wizard can end earlier. E.G. selecting No
on the first one doesn't ask for exempt end the other ones.
^^^ That one is probably related to some general encryption usage flag.
^^^ This one looks like usesNonExemptEncryption
, only its negated 😕
The rest of them looks like they are directly related to export_compliance_contains_proprietary_cryptography
, export_compliance_contains_third_party_cryptography
and export_compliance_available_on_french_store
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjovkovic Yeah, things look partially changed up. I think deprecating and using a new thing might just be best 😬 I was struggling to map everything with the new wordings 🤷♂️
Do you have an app that uses encryption? I don't have one so its really hard for me to test 😔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdholtz yes, I do :) It uses encryption, it's not exempt from export compliance, doesn't use any proprietary algorithms, it uses standard ones and it not available in French store. I can test when it's ready :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjovkovic That would be amazing! I've merged a bunch of work into the 2.150.0
branch if you are able to test that 👇 I'm planning on getting a bunch of new things out in a 2.150.0.rc2
either later tonight or early tomorrow 🤞
gem "fastlane", :git => "https://github.com/fastlane/fastlane.git", :branch => "2.150.0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdholtz I've tested again wih branch 2.150.0 in verbose mode. It failed again :(
INFO [2020-06-24 04:58:35.66]: Successfully selected build
DEBUG [2020-06-24 04:58:35.66]: Updating build for export compliance status of 'true'
DEBUG [2020-06-24 04:58:35.88]: Successfully updated build for export compliance status of '' on App Store Connect
DEBUG [2020-06-24 04:58:38.14]: Updating app store version for IDFA status of 'false'
DEBUG [2020-06-24 04:58:38.36]: Updated app store version for IDFA status of 'false'
INFO [2020-06-24 04:58:38.36]: Successfully updated IDFA delcarations on App Store Connect
DEBUG [2020-06-24 04:58:38.36]: Updating contents rights declaration on App Store Connect
INFO [2020-06-24 04:58:39.02]: Successfully updated contents rights declaration on App Store Connect
WARN [2020-06-24 04:58:39.87]: Lane Context:
INFO [2020-06-24 04:58:39.87]: {:DEFAULT_PLATFORM=>:ios, :PLATFORM_NAME=>:ios, :LANE_NAME=>"ios deploy", :VERSION_NUMBER=>"300000.2.30", :BUILD_NUMBER=>"300000230", :PRODUCE_APPLE_ID=>"1195204795", :CERT_FILE_PATH=>"******", :CERT_CERTIFICATE_ID=>"3Q2MB7V568", :SIGH_PROFILE_PATH=>"********", :SIGH_PROFILE_PATHS=>["********"], :SIGH_UDID=>"********", :SIGH_UUID=>"***********", :SIGH_NAME=>"*********", :SIGH_PROFILE_TYPE=>"app-store", :IPA_OUTPUT_PATH=>"***********", :XCODEBUILD_ARCHIVE=>"*************", :DSYM_OUTPUT_PATH=>"***********"}
ERROR [2020-06-24 04:58:39.87]: The request cannot be fulfilled because of the state of another resource. - Submit for review errors found.
The provided entity is missing a required attribute - You must provide a value for the attribute 'usesNonExemptEncryption' with this request
There's still a discrepancy with what's being set for usesNonExemptEncryption
and what is read afterwards from build.uses_non_exempt_encryption
. That looks to be the source of the problem.
DEBUG [2020-06-24 04:58:35.66]: Updating build for export compliance status of 'true'
DEBUG [2020-06-24 04:58:35.88]: Successfully updated build for export compliance status of '' on App Store Connect
Hey @joshdholtz 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
…ct API endpoints (#16640) * [deliver][produce][spaceship] Update to use new App Store Connect API endpoints (#16626) * Initial commit * Fixed ensuring version at start of deliver and selecting build at the end * Added error message for submission not working yet * Big fix for screenshots and fix for erroring out when not able to activate language * Removal of unused things and fix for explicitly submitting build * Fix for circular reference * Fix for first screenshot * content_rights_contains_third_party_content works for submission information * Updated comment * Quick fix * This should fix all bad state issues * Removed UI from spaceship * fixed submit stuff * Updating categories works holy cow * I think all meta data is working * Whoops. had submit for review commented out * Allowed rejected apps to be edit and only send up whatsNew if live version * rubocop is happy * So many fixes * Fixed validate doc and lint * Fixed and commented out some tests * Added in age rating * ruocoped * Adding back in IDFA because i dont know where it went * Fix produce for app store connect * Removed some pp * One extra line * Update to ConnectAPI usage Adding authorization and retrieving list of apps / find specific app. * [2.150.0][deliver] better error when uses non exempt encryption not set (#16646) * [deliver] fix for better error on uses non exempt encryption * New error messages and fixes of wrong things being set * [2.150.0][deliver] fix category deleting when not specified (#16652) * [deliver] fix category from deleting when not specified * Warn users of deprecated mapped categories * Updated some doc * [deliver] warn about mapped/deprecated age rating values (#16655) * [deliver] warn about mapped/deprecated age rating values * Full in on https * Lots of fixes whoops Co-authored-by: Max Ott <max.ott@me.com> * [deliver][produce] map language names and available countries on app (#16656) * [deliver][produce] map language names and available countries on app * Fix price match and remove comment * [deliver] allow default language for metadata (#16657) * [action] fix set_changelog (#16658) * [deliver][spaceship] multi-thread delete screenshots and retry on 504 (#16667) * [deliver][spaceship] multi-thread delete screenshots and retry on 504 * Improved error handling * Fix tests * [spaceship] add app preview (#16671) * [spaceship] add app preview * Added new preview devices * [spaceship] increase timeout for web session connect api to 1200 (#16676) * Version bump 2.150.0.rc2 * Fix to make produce create on ASC work again * [spaceship] waiting_for_review is editable (ish) (#16680) * [spaceship] waiting_for_review is editable (ish) * Added for app store version too * [spaceship] retry file uploader and poll app preview to set frame (#16684) * [spaceship] retry file uploader and poll app preview to set frame * Some doc cleanup * Handle fail logic if patch retries on file upload and fix mapping of age category * Raise error if all failed * [spaceship][fastlane_core] run itmstransporter with xcrun if xcode 11 or up (#16689) * [deliver][spaceship] fix 500 with appInfos and skip already uploaded screenshots with deliver (#16694) * Version bump to 2.150.0.rc4 * [produce][spaceship] add users to app when created (#16695) * [produce][spaceship] add users to app when created * Also add company name * Updated docs on how to use submission (#16706) * [deliver][spaceship] wait for screenshots to be completed after processing and download properly formatted screenshots and error on processing error (#16709) * [deliver][spaceship] choose highest edit version, retry on 500, submit without app version (#16713) * 2.150.0.rc5 bump * [deliver] give better error message when edit or live version is not found when downloading screenshots (#16714) * 2.150.0.rc6 bump * [deliver] fix first version logic now uses number of versions, reject_if_possible, and automatic_release_date uses ms again (#16715) * [deliver] fix first version logic now uses number of versions * Fix deliver reject and fix automatic release date to use time in ms * 2.150.0.rc7 bump * [spaceship] reorder app screenshots in AppScreenshotSet (#16718) * [deliver] fix deliver download screenshot file extension (#16719) * [deliver] fix deliver download screenshot file extension * Updated how skip waiting for processing works * [deliver] fix language detection on upload meta data (#16722) * Reset version to 2.149.0 * Fix for broken review attachment Co-authored-by: Max Ott <max.ott@me.com>
…ct API endpoints (fastlane#16640) * [deliver][produce][spaceship] Update to use new App Store Connect API endpoints (fastlane#16626) * Initial commit * Fixed ensuring version at start of deliver and selecting build at the end * Added error message for submission not working yet * Big fix for screenshots and fix for erroring out when not able to activate language * Removal of unused things and fix for explicitly submitting build * Fix for circular reference * Fix for first screenshot * content_rights_contains_third_party_content works for submission information * Updated comment * Quick fix * This should fix all bad state issues * Removed UI from spaceship * fixed submit stuff * Updating categories works holy cow * I think all meta data is working * Whoops. had submit for review commented out * Allowed rejected apps to be edit and only send up whatsNew if live version * rubocop is happy * So many fixes * Fixed validate doc and lint * Fixed and commented out some tests * Added in age rating * ruocoped * Adding back in IDFA because i dont know where it went * Fix produce for app store connect * Removed some pp * One extra line * Update to ConnectAPI usage Adding authorization and retrieving list of apps / find specific app. * [2.150.0][deliver] better error when uses non exempt encryption not set (fastlane#16646) * [deliver] fix for better error on uses non exempt encryption * New error messages and fixes of wrong things being set * [2.150.0][deliver] fix category deleting when not specified (fastlane#16652) * [deliver] fix category from deleting when not specified * Warn users of deprecated mapped categories * Updated some doc * [deliver] warn about mapped/deprecated age rating values (fastlane#16655) * [deliver] warn about mapped/deprecated age rating values * Full in on https * Lots of fixes whoops Co-authored-by: Max Ott <max.ott@me.com> * [deliver][produce] map language names and available countries on app (fastlane#16656) * [deliver][produce] map language names and available countries on app * Fix price match and remove comment * [deliver] allow default language for metadata (fastlane#16657) * [action] fix set_changelog (fastlane#16658) * [deliver][spaceship] multi-thread delete screenshots and retry on 504 (fastlane#16667) * [deliver][spaceship] multi-thread delete screenshots and retry on 504 * Improved error handling * Fix tests * [spaceship] add app preview (fastlane#16671) * [spaceship] add app preview * Added new preview devices * [spaceship] increase timeout for web session connect api to 1200 (fastlane#16676) * Version bump 2.150.0.rc2 * Fix to make produce create on ASC work again * [spaceship] waiting_for_review is editable (ish) (fastlane#16680) * [spaceship] waiting_for_review is editable (ish) * Added for app store version too * [spaceship] retry file uploader and poll app preview to set frame (fastlane#16684) * [spaceship] retry file uploader and poll app preview to set frame * Some doc cleanup * Handle fail logic if patch retries on file upload and fix mapping of age category * Raise error if all failed * [spaceship][fastlane_core] run itmstransporter with xcrun if xcode 11 or up (fastlane#16689) * [deliver][spaceship] fix 500 with appInfos and skip already uploaded screenshots with deliver (fastlane#16694) * Version bump to 2.150.0.rc4 * [produce][spaceship] add users to app when created (fastlane#16695) * [produce][spaceship] add users to app when created * Also add company name * Updated docs on how to use submission (fastlane#16706) * [deliver][spaceship] wait for screenshots to be completed after processing and download properly formatted screenshots and error on processing error (fastlane#16709) * [deliver][spaceship] choose highest edit version, retry on 500, submit without app version (fastlane#16713) * 2.150.0.rc5 bump * [deliver] give better error message when edit or live version is not found when downloading screenshots (fastlane#16714) * 2.150.0.rc6 bump * [deliver] fix first version logic now uses number of versions, reject_if_possible, and automatic_release_date uses ms again (fastlane#16715) * [deliver] fix first version logic now uses number of versions * Fix deliver reject and fix automatic release date to use time in ms * 2.150.0.rc7 bump * [spaceship] reorder app screenshots in AppScreenshotSet (fastlane#16718) * [deliver] fix deliver download screenshot file extension (fastlane#16719) * [deliver] fix deliver download screenshot file extension * Updated how skip waiting for processing works * [deliver] fix language detection on upload meta data (fastlane#16722) * Reset version to 2.149.0 * Fix for broken review attachment Co-authored-by: Max Ott <max.ott@me.com>
Where did this land? We are still seeing the error message "The provided entity is missing a required attribute - You must provide a value for the attribute 'usesNonExemptEncryption'" |
@AdamWheatley It looks like this was included in |
@joshdholtz I am also able to see this. |
@ppamorim Weird! Can you create an issue with your env and entire CLI output for me? 😇 |
Motivation and Context
#16645
Description
The error when "uses non exempt encryption" is not set isn't very helpful to the user. This should help them out 😊