Skip to content

Template examples do not have metadata:name field #7071

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

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

mburke5678
Copy link
Contributor

@mfojtik @bparees If this is in your area, please advise if these changes are correct:

I was working with the templates topic in the Dev Guide and was getting an error for a few examples:
The Template "" is invalid: metadata.name: Required value: name or generateName is required

I added a metadata:name field and the template was created.

Also, I got a YAML error on one example, and I needed to remove the \ in "{.data['my.username']}"

metadata:
    annotations:
      template.openshift.io/expose-username: "{.data['my\.username']}"

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 10, 2018
objects:
- kind: ConfigMap
apiVersion: v1
metadata:
annotations:
template.openshift.io/expose-username: "{.data['my\.username']}"
template.openshift.io/expose-username: "{.data['my.username']}"
Copy link
Contributor

Choose a reason for hiding this comment

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

i suspect the backslash was intentional/necessary. @jim-minter ?

Copy link
Contributor

Choose a reason for hiding this comment

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

(perhaps it needs to be double escaped, however)

@bparees
Copy link
Contributor

bparees commented Jan 10, 2018

need @jim-minter to weigh in on the blacklash, the rest seems ok to me.

@mburke5678
Copy link
Contributor Author

@bparees @jim-minter I get the following error with the \

oc create -f template-expose.yaml
error: yaml: line 9: found unknown escape character

@jim-minter
Copy link

Hold this for a little while please. The backslash is intentional, it is illustrating a comment in the document immediately above it. If doubled up, the backslash is accepted by oc create, but when you get the object back in yaml form via oc get, the backslash is single again - so I fear there's a client error (regression?) which needs to be dug out. I'll take a look in more detail tomorrow.

@jim-minter
Copy link

No client error (https://blog.plover.com/prog/compiler-error.html), and I've learned a new detail about yaml 😬

From http://www.yaml.org/spec/current.html#id2517668: "escape sequences are only interpreted in double quoted scalars. In all other scalar styles, the “\” character has no special meaning and non-printable characters are not available."

So OpenShift's behaviour here is correct; I got our documentation wrong.

@mburke5678 please could I ask you to:

  1. add an additional backslash so that the line in question reads template.openshift.io/expose-username: "{.data['my\\.username']}"

  2. append the following text to the paragraph directly above which ends "datum named `my.key`, the required JSONPath expression would be `{.data['my\.key']}`.":

Depending on how the JSONPath expression is then written in YAML, an additional backslash may be required, e.g. `"{.data['my\\.key']}"`.

Thanks!

@mburke5678
Copy link
Contributor Author

@jim-minter @bparees PTAL

@jim-minter
Copy link

/lgtm :)

@jim-minter
Copy link

thanks!

@mburke5678
Copy link
Contributor Author

@xiuwang Is this your area? Can you take a look at my changes?

@@ -671,12 +676,14 @@ The following is an example of different objects' fields being exposed:
----
kind: Template
apiVersion: v1
metadata:
name: my-template
objects:
- kind: ConfigMap
apiVersion: v1
metadata:
Copy link

Choose a reason for hiding this comment

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

Need define objects.metadata.name here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xiuwang PTAL

Copy link

Choose a reason for hiding this comment

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

@mburke5678 Other objects in this template also need add objects.metadata.name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xiuwang I added an objects.metadata.nameto each object in the template. Is name required? Theoc create -f`appears to have worked without the names.

Copy link

@xiuwang xiuwang Jan 22, 2018

Choose a reason for hiding this comment

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

@mburke5678 LGTM now
No error when 'oc create -f ' without objects.metadata.name, but will prompt error when 'oc new-app <above_template>'
error: ConfigMap "" is invalid: metadata.name: Required value: name or generateName is required
error: Secret "" is invalid: metadata.name: Required value: name or generateName is required
error: Service "" is invalid: metadata.name: Required value: name or generateName is required
error: Route "" is invalid: [metadata.name: Required value: name or generateName is required, spec.path: Invalid value: "mypath": path must begin with /, spec.to.name: Required value]

Copy link
Contributor

Choose a reason for hiding this comment

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

@jim-minter i think we should first determine which version of oc client it effects. "oc create -f" is effectively doing server side validation. "oc new-app" is effectively doing client side validation, so if you're using a 3.5 client and a 3.7 server, you're getting different validation versions.

Choose a reason for hiding this comment

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

More coffee needed. I don't think there's a bug here. oc create <template object> doesn't verify that objects[*].metadata.name exist (in any case, any could be parameterised, and the parameter could be left empty at process time). oc process <template object> | oc create -f - will error out if any object has no name. oc new-app is correctly effectively doing the latter.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

(I missed the nuance of what object was actually being created also)

Copy link

Choose a reason for hiding this comment

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

@mburke5678 Thanks all of you, LGTM now

@mburke5678
Copy link
Contributor Author

[rev_history]
|xref:../dev_guide/templates.adoc#dev-guide-templates[Templates]
|Corrected the template file example in the xref:../dev_guide/templates.adoc#writing-exposing-object-fields[Exposing Object Fields] section.
%

@mburke5678 mburke5678 added peer-review-needed Signifies that the peer review team needs to review this PR priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. branch/enterprise-3.6 branch/enterprise-3.7 branch/enterprise-3.9 branch/online branch/dedicated and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jan 23, 2018
@mburke5678
Copy link
Contributor Author

@openshift/team-documentation PTAL

@ahardin-rh
Copy link
Contributor

LGTM! 🎆

@ahardin-rh ahardin-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jan 23, 2018
@mburke5678 mburke5678 merged commit f988906 into openshift:master Jan 23, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 pushed a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
…BA-2017-31888-for-3-6

Added release notes for 3.6.173.0.96 async release xref:openshift#7071
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 pushed a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
….6-stage

[online-3.6] correct documented 'git clone' behaviour xref:openshift#7071
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 23, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 31, 2018
@mburke5678 mburke5678 deleted the template-errors branch August 2, 2018 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/dedicated branch/enterprise-3.6 branch/enterprise-3.7 branch/enterprise-3.9 branch/online peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants