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

Warn in the logs that a spec failed to load and continue #855

Merged
merged 1 commit into from Mar 28, 2018

Conversation

rthallisey
Copy link
Contributor

Describe what this PR does and why we need it:

Instead of failing when a spec fails to load, warn there was and error and output to debug the error.

Changes proposed in this pull request

  • No longer fail when we find a bad spec using crds

Instead of failing when a spec fails to load, warn there was
and error and output to debug the error.
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 26, 2018
@@ -102,10 +102,10 @@ func (d *Dao) BatchSetSpecs(specs apb.SpecManifest) error {
for id, spec := range specs {
err := d.SetSpec(id, spec)
if err != nil {
return err
log.Warningf("Error loading SPEC '%v'", spec.FQName)
Copy link
Member

Choose a reason for hiding this comment

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

Why not include the error in the warning? At the moment we don't have an excellent developer experience when an APB developer pushes up a bad APB. I would rather not require the broker to be in a debug mode in order to see useful error information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could, but the error I caught returns the contents of apb.yml plus the error. So I was thinking we may want to have this error only show if you opt in to loglevel: debug because it's quite noisy.

Here's the error:

[2018-03-26T13:45:20.589-04:00] [DEBUG] - SPEC 'dh-keycloak-apb' error: Bundle.automationbroker.io "b95513950bb3f132de25d58fb75f8dca" is invalid: []: Invalid value: map[string]interface {}{"spec":map[string]inte
rface {}{"metadata":"{\"dependencies\":[\"docker.io/jboss/keycloak-openshift:3.4.3.Final\",\"centos/postgresql-95-centos7:9.5\"],\"displayName\":\"Keycloak (APB)\",\"documentationUrl\":\"http://www.keycloak.org/
documentation.html\",\"imageUrl\":\"https://github.com/ansibleplaybookbundle/keycloak-apb/raw/master/docs/imgs/keycloak_ico.png\",\"providerDisplayName\":\"Red Hat, Inc.\",\"serviceName\":\"keycloak\"}", "plans"
:[]interface {}{map[string]interface {}{"description":"Deploy keycloak without persistence", "metadata":"{\"displayName\":\"Keycloak ephemeral\"}", "free":true, "bindable":false, "parameters":[]interface {}{map[
string]interface {}{"updatable":false, "title":"Keycloak admin username", "type":"string", "description":"", "default":"{\"default\":\"admin\"}", "deprecatedMaxLength":0, "name":"admin_username", "maxLength":0, 
"required":true}, map[string]interface {}{"type":"string", "description":"", "required":true, "deprecatedMaxLength":0, "maxLength":0, "updatable":false, "displayType":"password", "name":"admin_password", "title"
:"Keycloak admin password", "default":"{\"default\":null}"}, map[string]interface {}{"name":"apb_keycloak_uri", "title":"Keycloak URL", "description":"URL where the applications should redirect to for authentica
tion. Must be resolvable by the browser and pods. Leave empty to use the host generated by the route", "deprecatedMaxLength":0, "maxLength":0, "required":false, "type":"string", "default":"{\"default\":null}", "
updatable":false}, map[string]interface {}{"maxLength":0, "updatable":false, "displayType":"textarea", "title":"Users", "description":"JSON defining the users to add to the realm and their memberships", "default
":"{\"default\":null}", "deprecatedMaxLength":0, "name":"keycloak_users", "type":"string", "required":false}, map[string]interface {}{"default":"{\"default\":null}", "maxLength":0, "updatable":false, "displayTyp
e":"textarea", "name":"keycloak_roles", "title":"Roles", "type":"string", "description":"JSON defining the roles to add to the realm", "deprecatedMaxLength":0, "required":false}}, "bindParameters":[]interface {} {map[string]interface {}{"name":"service_name", "title":"Name of the service to bind", "type":"string", "description":"", "default":"{\"default\":null}", "deprecatedMaxLength":0, "maxLength":0, "require[32/1965]
"updatable":false, "displayGroup":"Provision"}, map[string]interface {}{"name":"redirect_uris", "title":"Redirect URIs", "deprecatedMaxLength":0, "maxLength":0, "required":true, "updatable":false, "type":"string
", "description":"Valid Redirect URIs a browser can redirect to after a successful login/logout. Simple wildcards are allowed. e.g. https://myservice-myproject.apps.example.com/*", "default":"{\"default\":null}"
, "displayGroup":"Provision"}, map[string]interface {}{"maxLength":0, "name":"web_origins", "title":"Web Origins", "type":"string", "required":false, "updatable":false, "displayGroup":"Provision", "description":
"Web Origins to allow CORS", "default":"{\"default\":null}", "deprecatedMaxLength":0}, map[string]interface {}{"name":"sso_url_name", "title":"Keycloak URL Variable name", "type":"string", "description":"How the
 application will refer to the Keycloak URL", "default":"{\"default\":\"SSO_URL\"}", "deprecatedMaxLength":0, "maxLength":0, "required":false, "updatable":false, "displayGroup":"Binding"}, map[string]interface {
}{"title":"Keycloak Realm Variable name", "description":"How the application will refer to the Keycloak Realm", "default":"{\"default\":\"SSO_REALM\"}", "deprecatedMaxLength":0, "name":"sso_realm_name", "maxLeng
th":0, "required":false, "updatable":false, "displayGroup":"Binding", "type":"string"}, map[string]interface {}{"name":"sso_client_name", "maxLength":0, "displayGroup":"Binding", "deprecatedMaxLength":0, "requir
ed":false, "updatable":false, "title":"Keycloak Client Variable name", "type":"string", "description":"How the application will refer to the Keycloak Client name", "default":"{\"default\":\"SSO_CLIENT\"}"}}, "id
":"a16a6598a77366d895e9a69a5b6e6b2c", "name":"ephemeral"}, map[string]interface {}{"description":"Deploy keycloak with persistence", "metadata":"{\"displayName\":\"Keycloak persistent\"}", "free":true, "bindable
":false, "parameters":[]interface {}{map[string]interface {}{"deprecatedMaxLength":0, "maxLength":0, "required":true, "updatable":false, "name":"admin_username", "title":"Keycloak admin username", "type":"string
", "description":"", "default":"{\"default\":\"admin\"}"}, map[string]interface {}{"description":"", "default":"{\"default\":null}", "displayType":"password", "maxLength":0, "required":true, "updatable":false, "
name":"admin_password", "title":"Keycloak admin password", "type":"string", "deprecatedMaxLength":0}, map[string]interface {}{"default":"{\"default\":null}", "required":false, "name":"apb_keycloak_uri", "type":"
string", "description":"URL where the applications should redirect to for authentication. Must be resolvable by the browser and pods. Leave empty to use the host generated by the route", "deprecatedMaxLength":0,
 "maxLength":0, "updatable":false, "title":"Keycloak URL"}, map[string]interface {}{"name":"pvc_size", "type":"string", "default":"{\"default\":\"200Mi\"}", "required":false, "title":"Storage size", "description
":"Database storage size", "deprecatedMaxLength":0, "maxLength":0, "updatable":false}, map[string]interface {}{"maxLength":0, "required":false, "updatable":false, "type":"string", "description":"JSON defining th
e users to add to the realm and their memberships", "default":"{\"default\":null}", "displayType":"textarea", "name":"keycloak_users", "title":"Users", "deprecatedMaxLength":0}, map[string]interface {}{"default"
:"{\"default\":null}", "required":false, "updatable":false, "name":"keycloak_roles", "type":"string", "description":"JSON defining the roles to add to the realm", "deprecatedMaxLength":0, "maxLength":0, "display
Type":"textarea", "title":"Roles"}}, "bindParameters":[]interface {}{map[string]interface {}{"description":"", "deprecatedMaxLength":0, "maxLength":0, "updatable":false, "displayGroup":"Provision", "name":"servi
ce_name", "title":"Name of the service to bind", "type":"string", "default":"{\"default\":null}", "required":true}, map[string]interface {}{"displayGroup":"Provision", "name":"redirect_uris", "title":"Redirect U
RIs", "default":"{\"default\":null}", "deprecatedMaxLength":0, "required":true, "updatable":false, "type":"string", "description":"Valid Redirect URIs a browser can redirect to after a successful login/logout. Simple wildcards are allowed. e.g. https://myservice-myproject.apps.example.com/*", "maxLength":0}, map[string]interface {}{"type":"string", "description":"Web Origins to allow CORS", "maxLength":0, "updatable":f
alse, "required":false, "displayGroup":"Provision", "name":"web_origins", "title":"Web Origins", "default":"{\"default\":null}", "deprecatedMaxLength":0}, map[string]interface {}{"default":"{\"default\":\"SSO_UR
L\"}", "deprecatedMaxLength":0, "maxLength":0, "required":false, "displayGroup":"Binding", "name":"sso_url_name", "title":"Keycloak URL Variable name", "type":"string", "description":"How the application will re
fer to the Keycloak URL", "updatable":false}, map[string]interface {}{"name":"sso_realm_name", "title":"Keycloak Realm Variable name", "type":"string", "maxLength":0, "required":false, "displayGroup":"Binding", 
"description":"How the application will refer to the Keycloak Realm", "default":"{\"default\":\"SSO_REALM\"}", "deprecatedMaxLength":0, "updatable":false}, map[string]interface {}{"title":"Keycloak Client Variab
le name", "deprecatedMaxLength":0, "maxLength":0, "required":false, "updatable":false, "name":"sso_client_name", "description":"How the application will refer to the Keycloak Client name", "default":"{\"default\
":\"SSO_CLIENT\"}", "displayGroup":"Binding", "type":"string"}}, "id":"07d919f45d0121a508b6e7b3be8502fb", "name":"persistent"}, map[string]interface {}{"parameters":[]interface {}{map[string]interface {}{"description":"", "type":"string", "title":"Keycloak admin username", "default":"{\"default\":\"admin\"}", "deprecatedMaxLength":0, "maxLength":0, "required":true, "updatable":false, "name":"admin_username"}, map[strin
g]interface {}{"type":"string", "maxLength":0, "displayType":"password", "default":"{\"default\":null}", "deprecatedMaxLength":0, "required":true, "updatable":false, "name":"admin_password", "title":"Keycloak ad
min password", "description":""}, map[string]interface {}{"name":"apb_keycloak_uri", "type":"string", "maxLength":0, "updatable":false, "title":"Keycloak URL", "description":"URL where the applications should re
direct to for authentication. Must be resolvable by the browser and pods.", "default":"{\"default\":null}", "deprecatedMaxLength":0, "required":true}, map[string]interface {}{"default":"{\"default\":null}", "upd
atable":false, "deprecatedMaxLength":0, "maxLength":0, "required":false, "displayType":"textarea", "name":"keycloak_users", "title":"Users", "type":"string", "description":"JSON defining the users to add to the 
realm and their memberships"}, map[string]interface {}{"name":"keycloak_roles", "title":"Roles", "description":"JSON defining the roles to add to the realm", "maxLength":0, "required":false, "displayType":"texta
rea", "type":"string", "default":"{\"default\":null}", "deprecatedMaxLength":0, "updatable":false}}, "bindParameters":[]interface {}{map[string]interface {}{"name":"service_name", "type":"string", "deprecatedMax
Length":0, "maxLength":0, "required":true, "title":"Name of the service to bind", "description":"", "default":"{\"default\":null}", "updatable":false, "displayGroup":"Provision"}, map[string]interface {}{"name":
"redirect_uris", "type":"string", "default":"{\"default\":null}", "required":true, "updatable":false, "title":"Redirect URIs", "description":"Valid Redirect URIs a browser can redirect to after a successful logi
n/logout. Simple wildcards are allowed. e.g. https://myservice-myproject.apps.example.com/*", "deprecatedMaxLength":0, "maxLength":0, "displayGroup":"Provision"}, map[string]interface {}{"required":false, "updat
able":false, "name":"web_origins", "type":"string", "description":"Web Origins to allow CORS", "default":"{\"default\":null}", "deprecatedMaxLength":0, "maxLength":0, "displayGroup":"Provision", "title":"Web Ori
gins"}, map[string]interface {}{"type":"string", "deprecatedMaxLength":0, "updatable":false, "required":false, "displayGroup":"Binding", "name":"sso_url_name", "title":"Keycloak URL Variable name", "description"
:"How the application will refer to the Keycloak URL", "default":"{\"default\":\"SSO_URL\"}", "maxLength":0}, map[string]interface {}{"name":"sso_realm_name", "title":"Keycloak Realm Variable name", "default":"{
\"default\":\"SSO_REALM\"}", "deprecatedMaxLength":0, "maxLength":0, "type":"string", "description":"How the application will refer to the Keycloak Realm", "required":false, "updatable":false, "displayGroup":"Bi
nding"}, map[string]interface {}{"default":"{\"default\":\"SSO_CLIENT\"}", "deprecatedMaxLength":0, "updatable":false, "name":"sso_client_name", "title":"Keycloak Client Variable name", "type":"string", "display
Group":"Binding", "description":"How the application will refer to the Keycloak Client name", "maxLength":0, "required":false}}, "id":"9344f7569606f7b01ab216b4c73d7aa5", "name":"external", "description":"Allows 
authenticating applications to an external Keycloak instance", "metadata":"{\"displayName\":\"Keycloak (external)\"}", "free":true, "bindable":false}}, "fq_name":"dh-keycloak-apb", "image":"docker.io/ansibleplay
bookbundle/keycloak-apb:latest", "tags":[]interface {}{"sso", "keycloak"}, "bindable":true, "runtime":2, "version":"1.0", "description":"Keycloak - Open Source Identity and Access Management", "async":"optional"
}, "kind":"Bundle", "apiVersion":"automationbroker.io/v1", "metadata":map[string]interface {}{"selfLink":"", "clusterName":"", "name":"b95513950bb3f132de25d58fb75f8dca", "namespace":"ansible-service-broker", "cr
eationTimestamp":"2018-03-26T17:45:20Z", "uid":"74106101-311d-11e8-b299-989096a334ed"}}: validation failure list:
spec.plans.bindParameters in body must be of type object: "array"

Copy link
Member

Choose a reason for hiding this comment

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

Ouch. You made the right choice.

Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

Pending the one comment on error info, LGTM.

Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknelson eriknelson merged commit 99d2332 into openshift:master Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants