Skip to content

Add ConsoleYAMLSample CRD type #465

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

Merged
openshift-merge-robot merged 1 commit into from
Oct 15, 2019
Merged

Add ConsoleYAMLSample CRD type #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions console/v1/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ func addKnownTypes(scheme *runtime.Scheme) error {
&ConsoleNotificationList{},
&ConsoleExternalLogLink{},
&ConsoleExternalLogLinkList{},
&ConsoleYAMLSample{},
&ConsoleYAMLSampleList{},
)
metav1.AddToGroupVersion(scheme, GroupVersion)
return nil
Expand Down
51 changes: 51 additions & 0 deletions console/v1/types_console_yaml_sample.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package v1

import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

// +genclient
// +genclient:nonNamespaced
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// ConsoleYAMLSample is an extension for customizing OpenShift web console YAML samples.
type ConsoleYAMLSample struct {
metav1.TypeMeta `json:",inline"`
// Standard object's metadata.
metav1.ObjectMeta `json:"metadata"`
Spec ConsoleYAMLSampleSpec `json:"spec"`
}

// ConsoleYAMLSampleSpec is the desired YAML sample configuration.
Copy link
Copy Markdown
Member

@spadgett spadgett Oct 11, 2019

Choose a reason for hiding this comment

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

Suggested change
// ConsoleYAMLSampleSpec is the desired YAML sample configuration.
// ConsoleYAMLSampleSpec is the desired YAML sample configuration.
// Samples will appear with their descriptions in a samples sidebar when creating a resources in the web console.

Copy link
Copy Markdown
Contributor

@benjaminapetersen benjaminapetersen Oct 11, 2019

Choose a reason for hiding this comment

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

How does this know what resource to reference in the console? I'm assuming its a customyamlsample for deployments, or another resource.

Copy link
Copy Markdown
Member

@spadgett spadgett Oct 11, 2019

Choose a reason for hiding this comment

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

Here is the proposal for target resource in the console PR: https://github.com/openshift/console/pull/2889/files#r331471061

Copy link
Copy Markdown
Contributor

@bparees bparees Oct 11, 2019

Choose a reason for hiding this comment

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

so is that going to be added to this ConsoleYAMLSample api?

Copy link
Copy Markdown
Member

@spadgett spadgett Oct 11, 2019

Choose a reason for hiding this comment

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

so is that going to be added to this ConsoleYAMLSample api?

Yes, we'll need that to know the kind of resource. The alternative is to try to parse the YAML sample content for all types to filter based on group/version/kind, but I'm not sure that's practical.

Copy link
Copy Markdown
Contributor

@bparees bparees Oct 11, 2019

Choose a reason for hiding this comment

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

well, the other option, if you foresee a need to reuse yaml samples for different resources, would be to have two different CRDs:

  1. yaml sample
  2. yaml sample reference/link to resource type to use it for

So you can create a single (1) and then multiple (2)s.

but i don't know if that is a valid use case or not. Or even if it's valid, if it would be common enough to be worth the effort over just creating duplicate ConsoleYamlSample objects.

Copy link
Copy Markdown
Member Author

@jhadvig jhadvig Oct 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not really sure what would be the use-case for reusing the CR for different resources.
If the would be one of the solutions would be as @spadgett suggested in comment to have TargetResource that would be an array

type ConsoleYAMLSampleSpec struct {
	TargetResource []metav1.TypeMeta `json:",targetResource"`
...

Thinking that if it wouldn't make more sense to have a multiple YAMLs for a single TargetResource ? Since we have multiple examples for some resources, eg. buildConfigs

Copy link
Copy Markdown
Member

@spadgett spadgett Oct 14, 2019

Choose a reason for hiding this comment

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

I'm not sure what the use case is for having more than one target resource since the API group / version / kind is also in the YAML. These should almost always match.

We do have one edge case in the UI where we show Roles and Cluster Roles in the same sidebar, but I think this pretty rare. There's always the workaround of creating more than one sample.

// Samples will appear with their descriptions in a samples sidebar
// when creating a resources in the web console.
type ConsoleYAMLSampleSpec struct {
Copy link
Copy Markdown
Member

@spadgett spadgett Oct 11, 2019

Choose a reason for hiding this comment

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

Looks like we're missing a way to specify the target resource apiVersion and kind

Copy link
Copy Markdown
Member Author

@jhadvig jhadvig Oct 14, 2019

Choose a reason for hiding this comment

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

metav1.TypeMeta contains apiVersion and kind that should target specific resource

Copy link
Copy Markdown
Contributor

@bparees bparees Oct 14, 2019

Choose a reason for hiding this comment

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

i think that field needs a more obvious name (rather than just inlining it) if that's how you're using it, to make it clear it's defining the resourcetype this sample applies to.

Copy link
Copy Markdown
Member Author

@jhadvig jhadvig Oct 14, 2019

Choose a reason for hiding this comment

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

Definitely, already added a comment in #465 (comment)

// targetResource contains apiVersion and kind of the resource
// YAML sample is representating.
TargetResource metav1.TypeMeta `json:",targetResource"`
// title of the YAML sample.
Title ConsoleYAMLSampleTitle `json:"title"`
// description of the YAML sample.
Description ConsoleYAMLSampleDescription `json:"description"`
// yaml is the YAML sample to display.
YAML ConsoleYAMLSampleYAML `json:"yaml"`
}

// ConsoleYAMLSampleTitle of the YAML sample.
// +kubebuilder:validation:Pattern=^(.|\s)*\S(.|\s)*$
type ConsoleYAMLSampleTitle string

// ConsoleYAMLSampleDescription of the YAML sample.
// +kubebuilder:validation:Pattern=^(.|\s)*\S(.|\s)*$
type ConsoleYAMLSampleDescription string

// ConsoleYAMLSampleYAML is the YAML sample to display.
// +kubebuilder:validation:Pattern=^(.|\s)*\S(.|\s)*$
type ConsoleYAMLSampleYAML string

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

type ConsoleYAMLSampleList struct {
metav1.TypeMeta `json:",inline"`
// Standard object's metadata.
metav1.ListMeta `json:"metadata"`
Items []ConsoleYAMLSample `json:"items"`
}
77 changes: 77 additions & 0 deletions console/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions console/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.