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

Move VPN ownership to menu agent #1690

Merged

Conversation

diegoreymendez
Copy link
Contributor

@diegoreymendez diegoreymendez commented Sep 28, 2023

Task/Issue URL: https://app.asana.com/0/72649045549333/1205529334042574/f

BSK PR: duckduckgo/BrowserServicesKit#536
iOS PR: duckduckgo/iOS#2101

Description

Moves the VPN ownership to the menu agent

Steps to test this PR

Apologies in advance for such a large PR, but this type of fundamental change is hard to split up further than this. I'll list the things that should be tested:

  • Test enabling the VPN from scratch, with the onboarding steps.
  • Test Debug menu > Reset Status
  • Test Waitlist onboarding flow.
  • Test controlling the VPN using the menu app.
  • Test notifications, and sending a test notification.

Some aids:
launchctl list | grep duck - to see our launchctl services
launchctl list remove com.duckduckgo.macos.vpn.debug to remove the VPN (debug) agent


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@diegoreymendez diegoreymendez changed the title Diego/feature/change netp vpn ownership prototype WIP: Change netp vpn ownership prototype Sep 28, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2023

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 123815e

@diegoreymendez diegoreymendez marked this pull request as ready for review October 17, 2023 10:26
@samsymons
Copy link
Collaborator

Tested the DMG workflows for release and review builds, those worked well 👍 Will do some final checks of this, then I think we can approve it.

Comment on lines -37 to -45
NETP_START_VPN_PROVISION_PROFILE_BASE64:
required: true
type: string
NETP_STOP_VPN_PROVISION_PROFILE_BASE64:
required: true
type: string
NETP_ENABLE_ON_DEMAND_PROVISION_PROFILE_BASE64:
required: true
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙌

"revision" : "57d8ed94cf503fdbd348420e821df00142660333",
"version" : "81.4.1"
"revision" : "4089fc0c70830e7cd6b3a574ae3654b365247ac3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding a reminder here that this will need to be fixed up before merging

Copy link
Collaborator

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

LGTM, nice work on this gargantuan change @diegoreymendez

diegoreymendez added a commit to duckduckgo/BrowserServicesKit that referenced this pull request Oct 22, 2023
Task/Issue URL: https://app.asana.com/0/0/1205738275545702/f

iOS PR: duckduckgo/iOS#2101
macOS PR: duckduckgo/macos-browser#1690

What kind of version bump will this require?: Major

## Description

Offers improved IPC support in BSK.
@diegoreymendez diegoreymendez merged commit ffeae3b into develop Oct 22, 2023
16 checks passed
@diegoreymendez diegoreymendez deleted the diego/feature/change-netp-vpn-ownership-prototype branch October 22, 2023 07:21
diegoreymendez added a commit to duckduckgo/iOS that referenced this pull request Oct 22, 2023
Task/Issue URL: https://app.asana.com/0/0/1205738275545703/f

BSK PR: duckduckgo/BrowserServicesKit#536
macOS PR: duckduckgo/macos-browser#1690

## Description

Integrates the latest changes from BSK.
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.

3 participants