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

feat: add user, system certificate in existing network security config #3724

Merged
merged 3 commits into from
Nov 17, 2024

Conversation

swngarg-lt
Copy link
Contributor

@swngarg-lt swngarg-lt commented Nov 11, 2024

With -n flag:
Current flow: creates a new network_security_config and updates in manifest
Updated flow: check if network_security_config exist, if not, create a new one (current approach) else modify network_security_config to add certificate without losing other rules added by user. i.e whitelisting any domain.

@swngarg-lt swngarg-lt changed the title add user, system certificate in existing network security config feat: add user, system certificate in existing network security config Nov 11, 2024
@swngarg-lt
Copy link
Contributor Author

swngarg-lt commented Nov 11, 2024

@iBotPeaches Can you review this?

@iBotPeaches
Copy link
Owner

Its only been 5 minutes lol. I can't work that fast.

Rather than comparing exact string, parse and check if user and system certificate exist
@iBotPeaches
Copy link
Owner

cc @erev0s as you added this feature. In case you have any feedback/comments on this addition.

@erev0s
Copy link
Contributor

erev0s commented Nov 13, 2024

thanks @iBotPeaches for the tag to check it.
@swngarg-lt nice improvement to cover the cases when you want to keep any existing network configuration. At this point I do not have the time to properly go through it and test for any edge cases, from a quick look it seems fine.

@swngarg-lt
Copy link
Contributor Author

thanks @iBotPeaches for the tag to check it. @swngarg-lt nice improvement to cover the cases when you want to keep any existing network configuration. At this point I do not have the time to properly go through it and test for any edge cases, from a quick look it seems fine.

So, @iBotPeaches @erev0s what can be the next action here?

@swngarg-lt swngarg-lt closed this Nov 15, 2024
@swngarg-lt swngarg-lt reopened this Nov 15, 2024
@iBotPeaches iBotPeaches merged commit 1eb1daf into iBotPeaches:master Nov 17, 2024
49 checks passed
@iBotPeaches
Copy link
Owner

thanks!

@iBotPeaches iBotPeaches added this to the v2.11.0 milestone Nov 17, 2024
iBotPeaches added a commit that referenced this pull request Nov 17, 2024
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