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

[Container App]: Add az containerapp ingress access-restriction group #5546

Merged
merged 32 commits into from
Nov 18, 2022

Conversation

runefa
Copy link
Contributor

@runefa runefa commented Nov 14, 2022


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

StrawnSC and others added 19 commits September 14, 2022 10:33
* initial no dockerfile support for up

* bump version

* add to help text, history text

* Fill in Cormac's link for oryx runtimes (see here microsoft/Oryx#1502)

* pass target port to ACR task; add better error information

* bump version

* fix bug with containerapp create

* add test case

* bump version

* Updated to preview api version and reran all tests. (Azure#147)

* Ran tests.

* Update src/containerapp/azext_containerapp/_clients.py

Co-authored-by: Silas Strawn <[email protected]>

Co-authored-by: Haroon Feisal <[email protected]>
Co-authored-by: Silas Strawn <[email protected]>

* resolve anthony's comments

* Revert "`az containerapp up`: Support No Dockerfile Scenario"

* Revert "Revert "`az containerapp up`: Support No Dockerfile Scenario""

* Added custom domain support to env create.

* Updated param help group text.

* Added test.

* Wrote env update, wrote test for env update with custom domains.

* Minor update.

* Added help to env update.

Co-authored-by: Silas Strawn <[email protected]>
Co-authored-by: Haroon Feisal <[email protected]>
Migrating containerapp-compose into containerapp
Revert "Migrating containerapp-compose into containerapp"
* Finished feature.

* Fixed comments.

Co-authored-by: Haroon Feisal <[email protected]>
@ghost ghost requested a review from yonzhan November 14, 2022 23:57
@ghost ghost added the Auto-Assign Auto assign by bot label Nov 14, 2022
@ghost ghost requested a review from wangzelin007 November 14, 2022 23:57
@ghost ghost assigned zhoxing-ms Nov 14, 2022
@ghost ghost added this to the Nov 2022 (2022-12-06) milestone Nov 14, 2022
@ghost ghost added the ContainerApp label Nov 14, 2022
@ghost ghost requested review from zhoxing-ms and jsntcy November 14, 2022 23:58
@yonzhan
Copy link
Collaborator

yonzhan commented Nov 15, 2022

Container App

@runefa
Copy link
Contributor Author

runefa commented Nov 16, 2022

@zhoxing-ms Could you take a look at this? Want to get this released soon if possible. Thanks

@@ -249,6 +249,12 @@ def load_arguments(self, _):
c.argument('target_port', type=int, validator=validate_target_port, help="The application port used for ingress traffic.")
c.argument('exposed_port', type=int, help="Additional exposed port. Only supported by tcp transport protocol. Must be unique per environment if the app ingress is external.")

with self.argument_context('containerapp ingress ip-restriction') as c:
c.argument('allow_access', help='Boolean indicating whether the ip security restriction allows or denies access.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we define this parameter as action='store_true' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter changes frequently, and we wouldn't want the default behavior to change (i.e. customers might create an ip restriction that allows when they want to block an ip range)

Copy link
Contributor

@zhoxing-ms zhoxing-ms Nov 18, 2022

Choose a reason for hiding this comment

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

@haroonf If the design of this parameter is not stable and has not been finalized, please add the is_preview flag to it.

@runefa runefa changed the title [Container App]: Add az containerapp ingress ip-restriction group [Container App]: Add az containerapp ingress access-restriction group Nov 17, 2022
src/containerapp/azext_containerapp/_params.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_params.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_params.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_params.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_help.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_help.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_help.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_help.py Outdated Show resolved Hide resolved
src/containerapp/azext_containerapp/_help.py Outdated Show resolved Hide resolved
@runefa
Copy link
Contributor Author

runefa commented Nov 18, 2022

@zhoxing-ms Could you take a look at this? Want to get this released soon if possible. Thanks

Let me know if there is anything else, thanks!

@zhoxing-ms zhoxing-ms merged commit 2bfa89a into Azure:main Nov 18, 2022
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ containerapp ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=15753&view=results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot ContainerApp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants