Skip to content

Commit

Permalink
feat: warn when trying to use unsupported devfile features
Browse files Browse the repository at this point in the history
Fix devfile#984

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
  • Loading branch information
AObuchow committed Dec 1, 2022
1 parent f8a7592 commit daff8c3
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 2 deletions.
162 changes: 162 additions & 0 deletions webhook/workspace/handler/warning.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
//
// Copyright (c) 2019-2022 Red Hat, Inc.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

package handler

import (
"fmt"
"strings"

dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
)

type unsupportedWarnings struct {
serviceAnnotations []string
endpointAnnotations []string
dedicatedPod []string
imageComponent []string
customComponent []string
volumeSize []string
eventPostStop []string
eventPreStop []string
}

func checkUnsupportedFeatures(devWorkspaceSpec dwv2.DevWorkspaceTemplateSpec) (warnings *unsupportedWarnings) {
warnings = &unsupportedWarnings{}
for _, component := range devWorkspaceSpec.Components {
if component.Container != nil {
if component.Container.Annotation != nil && component.Container.Annotation.Service != nil {
warnings.serviceAnnotations = append(warnings.serviceAnnotations, component.Name)
}
for _, endpoint := range component.Container.Endpoints {
if endpoint.Annotations != nil {
warnings.endpointAnnotations = append(warnings.endpointAnnotations, component.Name)
}
}
if component.Container.DedicatedPod != nil && *component.Container.DedicatedPod {
warnings.dedicatedPod = append(warnings.dedicatedPod, component.Name)
}
}

if component.Image != nil {
warnings.imageComponent = append(warnings.imageComponent, component.Name)
}
if component.Custom != nil {
warnings.customComponent = append(warnings.customComponent, component.Name)
}
if component.Volume != nil && component.Volume.Size != "" {
warnings.volumeSize = append(warnings.volumeSize, component.Name)
}
}

if devWorkspaceSpec.Events != nil {
if len(devWorkspaceSpec.Events.PostStop) > 0 {
warnings.eventPostStop = append(warnings.eventPostStop, devWorkspaceSpec.Events.PostStop...)
}

if len(devWorkspaceSpec.Events.PreStop) > 0 {
warnings.eventPreStop = append(warnings.eventPreStop, devWorkspaceSpec.Events.PreStop...)
}
}
return warnings
}

func unsupportedWarningsPresent(warnings *unsupportedWarnings) bool {
return len(warnings.serviceAnnotations) > 0 ||
len(warnings.endpointAnnotations) > 0 ||
len(warnings.dedicatedPod) > 0 ||
len(warnings.imageComponent) > 0 ||
len(warnings.customComponent) > 0 ||
len(warnings.volumeSize) > 0 ||
len(warnings.eventPostStop) > 0 ||
len(warnings.eventPreStop) > 0
}

func formatUnsupportedFeaturesWarning(warnings *unsupportedWarnings) string {
var msg []string
if len(warnings.serviceAnnotations) > 0 {
serviceAnnotationsMsg := "components[].container.annotation.service, used by components: " + strings.Join(warnings.serviceAnnotations, ", ")
msg = append(msg, serviceAnnotationsMsg)
}
if len(warnings.endpointAnnotations) > 0 {
endpointAnnotationsMsg := "components[].container.endpoints[].annotations, used by components: " + strings.Join(warnings.endpointAnnotations, ", ")
msg = append(msg, endpointAnnotationsMsg)
}
if len(warnings.dedicatedPod) > 0 {
dedicatedPodMsg := "components[].container.dedicatedPod, used by components: " + strings.Join(warnings.dedicatedPod, ", ")
msg = append(msg, dedicatedPodMsg)
}
if len(warnings.imageComponent) > 0 {
imageComponentMsg := "components[].image, used by components: " + strings.Join(warnings.imageComponent, ", ")
msg = append(msg, imageComponentMsg)
}
if len(warnings.customComponent) > 0 {
customComponentMsg := "components[].custom, used by components: " + strings.Join(warnings.customComponent, ", ")
msg = append(msg, customComponentMsg)
}
if len(warnings.volumeSize) > 0 {
volumeSizeMsg := "components[].volume.size, used by components: " + strings.Join(warnings.volumeSize, ", ")
msg = append(msg, volumeSizeMsg)
}
if len(warnings.eventPostStop) > 0 {
eventPostStopMsg := "events.postStop: " + strings.Join(warnings.eventPostStop, ", ")
msg = append(msg, eventPostStopMsg)
}
if len(warnings.eventPreStop) > 0 {
eventPreStopMsg := "events.preStop: " + strings.Join(warnings.eventPreStop, ", ")
msg = append(msg, eventPreStopMsg)
}
// TODO: Improve warning message?
return fmt.Sprintf("Unsupported Devfile features are present in this workspace. The following features will have no effect: %s", strings.Join(msg, "; "))
}

// Returns unsupported feature warnings that are present in the new workspace
// but not present in the new workspace
func checkForAddedUnsupportedFeatures(oldWksp, newWksp *dwv2.DevWorkspace) *unsupportedWarnings {
oldWarnings := checkUnsupportedFeatures(oldWksp.Spec.Template)
newWarnings := checkUnsupportedFeatures(newWksp.Spec.Template)
additionalWarnings := &unsupportedWarnings{}

// TODO: I don't really like the way 'oldWarningNames' and 'newWarningNames' are named
// since the warnings can be component names or events.
// They could be renamed to 'oldWarnings'/'newWarnings', but that could be confusing with
// the variables declared at the top of this function
unsupportedFeaturesDiff := func(oldWarningNames, newWarningNames []string) []string {
var additionalWarnings []string
for _, newWarning := range newWarningNames {
found := false
for _, oldWarning := range oldWarningNames {
if newWarning == oldWarning {
found = true
break
}
}
if !found {
additionalWarnings = append(additionalWarnings, newWarning)
}
}
return additionalWarnings
}

additionalWarnings.serviceAnnotations = append(additionalWarnings.serviceAnnotations, unsupportedFeaturesDiff(oldWarnings.serviceAnnotations, newWarnings.serviceAnnotations)...)
additionalWarnings.endpointAnnotations = append(additionalWarnings.endpointAnnotations, unsupportedFeaturesDiff(oldWarnings.endpointAnnotations, newWarnings.endpointAnnotations)...)
additionalWarnings.dedicatedPod = append(additionalWarnings.dedicatedPod, unsupportedFeaturesDiff(oldWarnings.dedicatedPod, newWarnings.dedicatedPod)...)
additionalWarnings.imageComponent = append(additionalWarnings.imageComponent, unsupportedFeaturesDiff(oldWarnings.imageComponent, newWarnings.imageComponent)...)
additionalWarnings.customComponent = append(additionalWarnings.customComponent, unsupportedFeaturesDiff(oldWarnings.customComponent, newWarnings.customComponent)...)
additionalWarnings.volumeSize = append(additionalWarnings.volumeSize, unsupportedFeaturesDiff(oldWarnings.volumeSize, newWarnings.volumeSize)...)
additionalWarnings.eventPreStop = append(additionalWarnings.eventPreStop, unsupportedFeaturesDiff(oldWarnings.eventPreStop, newWarnings.eventPreStop)...)
additionalWarnings.eventPostStop = append(additionalWarnings.eventPostStop, unsupportedFeaturesDiff(oldWarnings.eventPostStop, newWarnings.eventPostStop)...)
return additionalWarnings
}
22 changes: 20 additions & 2 deletions webhook/workspace/handler/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ func (h *WebhookHandler) MutateWorkspaceV1alpha2OnCreate(ctx context.Context, re
return admission.Denied(err.Error())
}

if warnings := checkUnsupportedFeatures(wksp.Spec.Template); unsupportedWarningsPresent(warnings) {
h.returnPatched(req, wksp).WithWarnings(formatUnsupportedFeaturesWarning(warnings))
}

return h.returnPatched(req, wksp)
}

Expand Down Expand Up @@ -107,6 +111,12 @@ func (h *WebhookHandler) MutateWorkspaceV1alpha2OnUpdate(ctx context.Context, re
return admission.Denied("DevWorkspace ID cannot be changed once it is set")
}

warnings := ""
addedUnsupportedFeatures := checkForAddedUnsupportedFeatures(oldWksp, newWksp)
if unsupportedWarningsPresent(addedUnsupportedFeatures) {
warnings = formatUnsupportedFeaturesWarning(addedUnsupportedFeatures)
}

// TODO: re-enable webhooks for storageClass once handling is improved.
// oldStorageType := oldWksp.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil)
// newStorageType := newWksp.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil)
Expand Down Expand Up @@ -155,14 +165,22 @@ func (h *WebhookHandler) MutateWorkspaceV1alpha2OnUpdate(ctx context.Context, re
newWksp.Labels = map[string]string{}
}
newWksp.Labels[constants.DevWorkspaceCreatorLabel] = oldCreator
return h.returnPatched(req, newWksp)
response := h.returnPatched(req, newWksp)
if warnings != "" {
return response.WithWarnings(warnings)
}
return response
}

if newCreator != oldCreator {
return admission.Denied(fmt.Sprintf("label '%s' is assigned once devworkspace is created and is immutable", constants.DevWorkspaceCreatorLabel))
}

return admission.Allowed("new workspace has the same devworkspace as old one")
response := admission.Allowed("new workspace has the same devworkspace as old one")
if warnings != "" {
response.WithWarnings(warnings)
}
return response
}

func hasFinalizer(obj client.Object, finalizer string) bool {
Expand Down

0 comments on commit daff8c3

Please sign in to comment.