Bug 1201615 - Give swagger the correct base URL #1780
Conversation
|
@deads2k review |
| @@ -47,10 +47,14 @@ type MasterConfig struct { | |||
| // CORSAllowedOrigins | |||
| CORSAllowedOrigins []string | |||
|
|
|||
| // MasterPublicURL is how clients can access the OpenShift API server | |||
| MasterPublicURL string | |||
There was a problem hiding this comment.
Add validation to ensure the URL is present (required for swagger), well-formed, and matches any other MasterPublicURLs that may be present.
There was a problem hiding this comment.
Oh, and for the next question: each stanza has its own copy to keep it contained if we decide to allow starting these components separately.
There was a problem hiding this comment.
@liggitt PublicURL to match the AssetConfig?
There was a problem hiding this comment.
We don't validate well formedness of the others.
Also, why do they have to match?
On Apr 17, 2015, at 8:12 AM, David Eads notifications@github.com wrote:
In pkg/cmd/server/api/types.go:
@@ -47,10 +47,14 @@ type MasterConfig struct {
// CORSAllowedOrigins
CORSAllowedOrigins []string
- // MasterPublicURL is how clients can access the OpenShift API server
- MasterPublicURL string
Add validation to ensure the URL is present (required for swagger), well-formed, and matches any other MasterPublicURLs that may be present.—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
MasterConfig has no effective stanza. What would you suggest.
On Apr 17, 2015, at 8:14 AM, David Eads notifications@github.com wrote:
In pkg/cmd/server/api/types.go:
@@ -47,10 +47,14 @@ type MasterConfig struct {
// CORSAllowedOrigins
CORSAllowedOrigins []string
- // MasterPublicURL is how clients can access the OpenShift API server
- MasterPublicURL string
Oh, and for the next question: each stanza has its own copy to keep it contained if we decide to allow starting these components separately.—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
PublicURLto match theAssetConfig?
I can't decide whether I like everything using the same names to refer to the same things, or removing prefixes based on context. I think I lean toward the same names... but I don't feel that strongly
There was a problem hiding this comment.
Same name. Less mental struggle.
----- Original Message -----
@@ -47,10 +47,14 @@ type MasterConfig struct {
// CORSAllowedOrigins
CORSAllowedOrigins []string
- // MasterPublicURL is how clients can access the OpenShift API server
- MasterPublicURL string
PublicURLto match theAssetConfig?I can't decide whether I like everything using the same names to refer to the
same things, or removing prefixes based on context. I think I lean toward
the same names... but I don't feel that strongly
Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/1780/files#r28603240
There was a problem hiding this comment.
We're not validating those URLs today. Why?
----- Original Message -----
We don't validate well formedness of the others.
Also, why do they have to match?
On Apr 17, 2015, at 8:12 AM, David Eads notifications@github.com wrote:
In pkg/cmd/server/api/types.go:
@@ -47,10 +47,14 @@ type MasterConfig struct {
// CORSAllowedOrigins
CORSAllowedOrigins []string
- // MasterPublicURL is how clients can access the OpenShift API server
- MasterPublicURL string
Add validation to ensure the URL is present (required for swagger),
well-formed, and matches any other MasterPublicURLs that may be present.—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
We're not validating those URLs today. Why?
Oversight?
smarterclayton
force-pushed
the
fix_1201615_https_in_swagger
branch
from
ddf3fd9
to
f53feee
Compare
|
Added unit tests. |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1608/) (Image: devenv-fedora_1297) |
|
Evaluated for origin up to f53feee |
…gger Merged by openshift-bot
…service-catalog/' changes from c3e3071633..231772fcc0 231772fcc0 origin build: add origin tooling 98af588 v0.1.11 release changes 01e2f90 v0.1.10 release changes 49af948 clear polling queue before starting new operation (openshift#1855) 252958e Refactor common serviceclass validations (openshift#1858) 68f55c6 Catalog Controller should listen on https and serve metrics over TLS secured channel (openshift#1851) 5d0f773 Refactor common broker validations (openshift#1865) eeaf285 Add NamespacedServiceBroker switch to helm chart (openshift#1864) d2c960c Add NamespacedServiceBroker Feature (openshift#1863) ef15310 Fix NamespaceScoped doc text for ns types (openshift#1862) 8d0a637 fix async deprovision retry (openshift#1832) a918a16 Update registry code from serviceclass to clusterserviceclass (openshift#1852) 4dfd13c Bump dependency on go-open-service-broker-client to 0.0.6 (openshift#1856) 958b7cd Rename SharedServicePlanSpec to CommonServicePlanSpec (openshift#1850) 426aec3 pick a better random port to listen on in integration tests (openshift#1844) 6a59ada OrphanMitigation condition and different handling of retry timeout (openshift#1789) c9b8f60 Extracting common broker spec elements into embeddable struct (openshift#1841) 3f8fab6 Extract common service class spec fields (openshift#1834) 93dab13 Support for OSB [PR#452](openservicebrokerapi/servicebroker#452). (openshift#1849) 5e1e90d [WIP] Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1847) 74f73c0 disable tests for deployment stage (openshift#1845) 82fc6e4 Revert "Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803)" (openshift#1843) 94b5795 gitignore integration.test binary (openshift#1840) 014c468 Extracting common plan spec into embeddable struct (openshift#1833) 5d7041b Use k8s NewUUID method exclusively (openshift#1836) eac3f96 A new test was added after prechecks happened for last pr. (openshift#1838) 4b5d159 Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803) cc02f0e [svcat] Adding a filter to get plan. (openshift#1758) 70afb56 reset RemovedFromBroker flag on plans that are re-added by broker (openshift#1824) 712dd4a Add behavior to print deleted instance name (openshift#1806) 55505be Update catalog charts README configuration (openshift#1823) 6426c98 Controller reconciliation rework - part 2 (ServiceBinding) (openshift#1819) c606560 Integrate svcat docs with Service Catalog's (openshift#1784) e9aeeb0 Synchronize some generated code that was missed along the way (openshift#1801) a63ebf7 Fix failing test: TestReconcileServiceInstanceWithFailedCondition (openshift#1813) 07ef743 Controller reconciliation rework - part 1 (ServiceInstance) (openshift#1779) a7d602b Export the touch instance command (openshift#1809) bddb9a7 Allow retries for instances with Failed condition after spec changes (openshift#1751) a777af5 Add enhanced parameter options to provision (openshift#1785) fd1a0b9 Print deleted bindings (openshift#1799) 36d437a Adding the ability to sync a service instance (openshift#1762) 1f60676 Remove Failed condition if there was no terminal failure (openshift#1788) cd831de allow brokers to respond to getCatalog() with no services (openshift#1772) (openshift#1781) e5c37ad Add ObservedGeneration and Provisioned into ServiceInstanceStatus (openshift#1748) 9021d8b Add carolynvs to OWNERS (openshift#1780) b7643d6 Add all-namespaces flag to svcat (openshift#1782) 01e652f Use docker to interact with files made by docker (openshift#1777) REVERT: c3e3071633 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 231772fcc00be08b6b2665a39c4a3bacb0b2271f
…service-catalog/' changes from c3e3071633..231772fcc0 231772fcc0 origin build: add origin tooling 98af588 v0.1.11 release changes 01e2f90 v0.1.10 release changes 49af948 clear polling queue before starting new operation (openshift#1855) 252958e Refactor common serviceclass validations (openshift#1858) 68f55c6 Catalog Controller should listen on https and serve metrics over TLS secured channel (openshift#1851) 5d0f773 Refactor common broker validations (openshift#1865) eeaf285 Add NamespacedServiceBroker switch to helm chart (openshift#1864) d2c960c Add NamespacedServiceBroker Feature (openshift#1863) ef15310 Fix NamespaceScoped doc text for ns types (openshift#1862) 8d0a637 fix async deprovision retry (openshift#1832) a918a16 Update registry code from serviceclass to clusterserviceclass (openshift#1852) 4dfd13c Bump dependency on go-open-service-broker-client to 0.0.6 (openshift#1856) 958b7cd Rename SharedServicePlanSpec to CommonServicePlanSpec (openshift#1850) 426aec3 pick a better random port to listen on in integration tests (openshift#1844) 6a59ada OrphanMitigation condition and different handling of retry timeout (openshift#1789) c9b8f60 Extracting common broker spec elements into embeddable struct (openshift#1841) 3f8fab6 Extract common service class spec fields (openshift#1834) 93dab13 Support for OSB [PR#452](openservicebrokerapi/servicebroker#452). (openshift#1849) 5e1e90d [WIP] Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1847) 74f73c0 disable tests for deployment stage (openshift#1845) 82fc6e4 Revert "Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803)" (openshift#1843) 94b5795 gitignore integration.test binary (openshift#1840) 014c468 Extracting common plan spec into embeddable struct (openshift#1833) 5d7041b Use k8s NewUUID method exclusively (openshift#1836) eac3f96 A new test was added after prechecks happened for last pr. (openshift#1838) 4b5d159 Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803) cc02f0e [svcat] Adding a filter to get plan. (openshift#1758) 70afb56 reset RemovedFromBroker flag on plans that are re-added by broker (openshift#1824) 712dd4a Add behavior to print deleted instance name (openshift#1806) 55505be Update catalog charts README configuration (openshift#1823) 6426c98 Controller reconciliation rework - part 2 (ServiceBinding) (openshift#1819) c606560 Integrate svcat docs with Service Catalog's (openshift#1784) e9aeeb0 Synchronize some generated code that was missed along the way (openshift#1801) a63ebf7 Fix failing test: TestReconcileServiceInstanceWithFailedCondition (openshift#1813) 07ef743 Controller reconciliation rework - part 1 (ServiceInstance) (openshift#1779) a7d602b Export the touch instance command (openshift#1809) bddb9a7 Allow retries for instances with Failed condition after spec changes (openshift#1751) a777af5 Add enhanced parameter options to provision (openshift#1785) fd1a0b9 Print deleted bindings (openshift#1799) 36d437a Adding the ability to sync a service instance (openshift#1762) 1f60676 Remove Failed condition if there was no terminal failure (openshift#1788) cd831de allow brokers to respond to getCatalog() with no services (openshift#1772) (openshift#1781) e5c37ad Add ObservedGeneration and Provisioned into ServiceInstanceStatus (openshift#1748) 9021d8b Add carolynvs to OWNERS (openshift#1780) b7643d6 Add all-namespaces flag to svcat (openshift#1782) 01e652f Use docker to interact with files made by docker (openshift#1777) REVERT: c3e3071633 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 231772fcc00be08b6b2665a39c4a3bacb0b2271f
No description provided.