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

Add Istio k8s client #80

Merged
merged 3 commits into from
Mar 6, 2019
Merged

Add Istio k8s client #80

merged 3 commits into from
Mar 6, 2019

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan commented Mar 5, 2019

This PR replaces the knative pkg with an Istio client generated based on the v1.1.0 protos. Only the Virtual Service CRD is implemented from the networking/v1alpha3 group.

Fix: #79

CORS and RetryOn are missing from the knative pkg.
Until Istio has an official k8s client, we'll maintain our own.
@codecov-io
Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #80 into master will increase coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   54.53%   54.56%   +0.03%     
==========================================
  Files          13       13              
  Lines        1533     1532       -1     
==========================================
  Hits          836      836              
+ Misses        557      556       -1     
  Partials      140      140
Impacted Files Coverage Δ
pkg/controller/deployer.go 58.7% <ø> (ø) ⬆️
pkg/router/istio.go 80% <ø> (ø) ⬆️
pkg/controller/controller.go 7% <ø> (+0.04%) ⬆️
pkg/router/factory.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eda97f3...a8d8bb2. Read the comment docs.

@@ -0,0 +1,19 @@
package v1alpha1
Copy link
Contributor

Choose a reason for hiding this comment

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

Did this get generated or you did this by hand? this is for my learning 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

All was done by hand :(

@huydinhle
Copy link
Contributor

huydinhle commented Mar 6, 2019

👍 This is nice. I assume this will solve our problem with CORS policy(#58) that is unsupported currently in the CanaryService object as well ?

@stefanprodan
Copy link
Member Author

Yes this will allow us to ship CORS support. What's more important is that we can now control the upgrade path to Istio 1.1 without depending on some other project that could chose to delay the upgrade for months.

@stefanprodan stefanprodan merged commit e094c2a into master Mar 6, 2019
@stefanprodan stefanprodan deleted the istio branch March 6, 2019 09:55
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