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.

Sign up for GitHub

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

Jump to bottom

API controller: add API validation - allow one API per service #2621

Merged
merged 12 commits into from Feb 8, 2019
Merged

API controller: add API validation - allow one API per service #2621

merged 12 commits into from Feb 8, 2019

Conversation

jakkab
Copy link
Contributor

@jakkab jakkab commented Feb 6, 2019

Description

Changes proposed in this pull request:

  • api-controller validates if an API already exists for a specified Kubernetes service
  • tests

@jakkab jakkab added kind/bug Categorizes issue or PR as related to a bug. WIP area/security Issues or PRs related to security area/service-mesh Issues or PRs related to service-mesh labels Feb 6, 2019
@jakkab jakkab added this to the Sprint_Goat_5 milestone Feb 6, 2019
@CLAassistant
Copy link

CLAassistant commented Feb 6, 2019 •
edited

CLA assistant check
All committers have signed the CLA.

@jakkab jakkab mentioned this pull request Feb 7, 2019
Merged
@jakkab jakkab removed the WIP label Feb 7, 2019
@jakkab jakkab mentioned this pull request Feb 7, 2019
Merged
@piotrmsc
Copy link

piotrmsc commented Feb 8, 2019 •
edited

If you create an API for a service that does not have API attached but you have selected wrong service and make an update to correct one, Virtual service and Policy(if set) should be deleted for the wrong service (the first selected) because you will leave garbage that may have big impact on how other services work.

Not in scope of this task : we should have a blacklist of service like kubernetes for which api must not be created. Can you create a follow up for that?

@piotrmsc
Copy link

piotrmsc commented Feb 8, 2019 •
edited

Please extend our api-controller-acceptnce-tests with assertions for validationStatus.
Please also create an integration test for scenarios

  • I create an API for service already exposed
  • I update an API for service already exposed

And while chaning that please consider making this line
configurable via env like it is with domain for example

@jakkab
Copy link
Contributor Author

jakkab commented Feb 8, 2019

@piotrmsc Virtual Services and Polices are updated as well, no need to do delete anything.

Follow-up will be created.

@jakkab jakkab merged commit bc12020 into kyma-project:master Feb 8, 2019
@jakkab jakkab deleted the validate-api-for-service branch February 8, 2019 15:02
grischperl pushed a commit to grischperl/kyma that referenced this pull request Nov 10, 2020
@Ressetkk
Add new testgrid config annotation to the docs (kyma-project#2621)
d4eeef6 
* Add new testgrid config annotation to te docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piotrmsc piotrmsc piotrmsc left review comments

@sjanota sjanota sjanota approved these changes

@kfurgol kfurgol kfurgol approved these changes

@Demonsthere Demonsthere Awaiting requested review from Demonsthere

@kubadz kubadz Awaiting requested review from kubadz

@strekm strekm Awaiting requested review from strekm

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP Awaiting requested review from Tomasz-Smelcerz-SAP

Assignees

@sjanota sjanota

@jakkab jakkab

Labels
area/security Issues or PRs related to security area/service-mesh Issues or PRs related to service-mesh kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jakkab @CLAassistant @piotrmsc @sjanota @kfurgol