Feat/roadmap by julius-malcovsky · Pull Request #302 · telekom/controlplane

julius-malcovsky · 2026-03-30T20:43:19Z

Please see /docs for details - after the review we can remove these files.

…nts etc Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

Copilot

Pull request overview

This PR implements a comprehensive Roadmap Custom Resource Definition (CRD) feature that provides a generic, Kubernetes-native way to track timeline and roadmap information for various resource types (APIs, Events, etc.). The implementation spans both the Kubernetes operator layer (rover/) and the REST API server layer (rover-server/), with full webhook validation, controllers, handlers, and extensive test coverage.

Changes:

Adds Roadmap CRD type definitions with ResourceType enum supporting "API" and "Event" types
Implements Kubernetes operator components (controller, handler, validation webhook) for managing Roadmap resources
Adds comprehensive REST API layer with CRUD operations, file-manager integration, and hash-based optimization
Includes thorough test coverage across all layers with working tests passing
Updates documentation with feature specifications and migration guide

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rover/api/v1/roadmap_types.go	CRD type definitions (ResourceType, RoadmapSpec, RoadmapStatus)
rover/internal/controller/roadmap_controller.go	Kubernetes reconciliation controller following generic pattern
rover/internal/handler/roadmap/handler.go	Minimal handler that sets conditions for successful reconciliation
rover/internal/webhook/v1/roadmap_webhook.go	Validation webhook ensuring required fields and enum values
rover-server/internal/controller/roadmap.go	REST API controller with CRUD operations and file-manager integration
rover-server/internal/server/roadmap_server.go	HTTP request/response handlers and routing
rover/README.md	Updated feature list with Roadmap documentation
rover-server/api/openapi.yaml	OpenAPI specification with Roadmap endpoints (also fixes typo: "types:" → "type:")
docs/	Comprehensive feature documentation and implementation summary

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T20:46:10Z

rover-server/internal/controller/roadmap.go

+// Update updates an existing roadmap
+func (r *RoadmapController) Update(ctx context.Context, resourceId string, req api.RoadmapRequest) (res api.RoadmapResponse, err error) {
+	// Validate request
+	if req.ResourceName == "" {
+		return res, problems.BadRequest("resourceName must not be empty")
+	}
+	if req.ResourceType != api.RoadmapResourceTypeAPI && req.ResourceType != api.RoadmapResourceTypeEvent {
+		return res, problems.BadRequest("resourceType must be either 'API' or 'Event'")
+	}
+	if len(req.Items) == 0 {
+		return res, problems.BadRequest("items array must contain at least one item")
+	}
+
+	id, err := mapper.ParseResourceId(ctx, resourceId)
+	if err != nil {
+		return res, err
+	}
+
+	// Marshal items to JSON
+	itemsMarshaled, err := json.Marshal(req.Items)
+	if err != nil {
+		return res, problems.BadRequest("failed to marshal items: " + err.Error())
+	}
+
+	// Upload to file-manager
+	fileAPIResp, err := r.uploadFile(ctx, itemsMarshaled, id)
+	if err != nil {
+		return res, err
+	}
+
+	// Create/Update CRD (declarative PUT - creates if not exists, updates if exists)
+	ns := id.Environment + "--" + id.Namespace
+	roadmap := &roverv1.Roadmap{}
+	roadmap.TypeMeta = metav1.TypeMeta{
+		Kind:       "Roadmap",
+		APIVersion: "rover.cp.ei.telekom.de/v1",
+	}
+	roadmap.Name = id.Name
+	roadmap.Namespace = ns
+	roadmap.Spec.ResourceName = req.ResourceName
+	roadmap.Spec.ResourceType = roverv1.ResourceType(req.ResourceType)
+	roadmap.Spec.Roadmap = fileAPIResp.FileId
+	roadmap.Spec.Hash = fileAPIResp.FileHash
+	EnsureLabelsOrDie(ctx, roadmap)
+
+	err = r.Store.CreateOrReplace(ctx, roadmap)
+	if err != nil {
+		return res, err
+	}
+
+	return r.Get(ctx, resourceId)
+}


The removeDuplicates() method is defined in this controller but is never called in the Update() method. According to the implementation documentation, the system should "automatically ensure one roadmap per resource" by removing old roadmaps with the same resourceName + resourceType combination when updating via PUT. However, the Update method creates or replaces the CRD without invoking this duplicate removal logic, meaning multiple roadmaps with the same resourceName and resourceType could coexist, violating the stated requirement.

Copilot · 2026-03-30T20:46:11Z

rover-server/internal/controller/roadmap.go

+	return api.RoadmapResponse{
+		Id:           id.ResourceId,
+		Name:         roadmap.Name,
+		ResourceName: roadmap.Spec.ResourceName,
+		ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),
+		Items:        items,
+		Status:       *mapStatus(roadmap),
+	}, nil


The mapStatus() function can return nil (line 341) when the Roadmap has no conditions, but line 148 dereferences the result with *mapStatus(roadmap) without a nil check. This will cause a runtime panic if the roadmap has no status conditions set (which could happen for newly created roadmaps before the controller reconciles them).

Suggested change

return api.RoadmapResponse{

Id: id.ResourceId,

Name: roadmap.Name,

ResourceName: roadmap.Spec.ResourceName,

ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),

Items: items,

Status: *mapStatus(roadmap),

}, nil

resp := api.RoadmapResponse{

Id: id.ResourceId,

Name: roadmap.Name,

ResourceName: roadmap.Spec.ResourceName,

ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),

Items: items,

}

if status := mapStatus(roadmap); status != nil {

resp.Status = *status

}

return resp, nil

Copilot · 2026-03-30T20:46:11Z

rover-server/internal/controller/roadmap.go

+		resp := api.RoadmapResponse{
+			Id:           resourceId,
+			Name:         roadmap.Name,
+			ResourceName: roadmap.Spec.ResourceName,
+			ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),
+			Items:        items,
+			Status:       *mapStatus(roadmap),


Same nil pointer dereference issue as in the Get() method: line 206 dereferences *mapStatus(roadmap) without checking for nil. If any roadmap in the list lacks status conditions, this will panic.

Suggested change

resp := api.RoadmapResponse{

Id: resourceId,

Name: roadmap.Name,

ResourceName: roadmap.Spec.ResourceName,

ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),

Items: items,

Status: *mapStatus(roadmap),

// Map status safely, guarding against nil pointers

mappedStatus := mapStatus(roadmap)

var status api.RoadmapStatus

if mappedStatus != nil {

status = *mappedStatus

}

resp := api.RoadmapResponse{

Id: resourceId,

Name: roadmap.Name,

ResourceName: roadmap.Spec.ResourceName,

ResourceType: api.RoadmapResponseResourceType(roadmap.Spec.ResourceType),

Items: items,

Status: status,

ron96g

Will review further. For now, some open questions.

ron96g · 2026-04-01T08:21:30Z

docs/roadmap-feature.md

@@ -0,0 +1,450 @@
+# Roadmap CRD Feature


we decided not to include these design and plan markdown files in MR to main. Remove before merging

ron96g · 2026-04-01T08:28:16Z

rover/api/v1/roadmap_types.go

+}
+
+// RoadmapSpec defines the desired state of Roadmap
+type RoadmapSpec struct {


Why not use types.TypedObjectRef instead of ResourceName and ResourceType? That way we directly reference the EventSpecification or ApiSpecification CR?

ron96g · 2026-04-01T08:28:45Z

rover/api/v1/roadmap_types.go

+// The actual items are stored in file-manager as JSON
+type RoadmapItem struct {
+	// Date of the roadmap item (e.g., "Q1 2024", "2024-03-15")
+	Date string `json:"date"`


string vs metav1.Time. However, then "Q1 2024" and similiar strings would not be supported. So probably string is the better choice to avoid breaking changes

ron96g · 2026-04-01T08:30:09Z

rover-server/api/openapi.yaml

  - name: Rover
    description: Manage Rovers
+  - name: Roadmap
+    description: Manage Roadmaps


How to handle the old ApiRoadmap and ApiChangelog? They were removed from the openapi-spec previously because we did not support them. Now we have to decide: Do we want to add them back or just keep Roadmap + Changelog? That would be a breaking change

ron96g · 2026-04-01T08:31:09Z

rover-server/internal/types/roadmap.go

+	roverv1 "github.com/telekom/controlplane/rover/api/v1"
+)
+
+// RoadmapItem represents a single timeline entry in the roadmap


Why is this needed? Arent both sides openapi-generated and Custom-Resource already defined? What are these types used for?

ron96g · 2026-04-01T08:33:23Z

rover/internal/webhook/v1/roadmap_webhook.go

+	return nil, nil
+}
+
+func (v *RoadmapCustomValidator) ValidateCreateOrUpdate(ctx context.Context, roadmap *roverv1.Roadmap) (admission.Warnings, error) {


If a Roadmap references an ApiSpecification, which does not exist. How should this be handled?

ron96g · 2026-04-01T08:33:55Z

rover/README.md

+<details>
+<summary>
+<strong>EventSpecification</strong>
+This CRD represents an AsyncAPI specification for an Event type.


What is this? This is wrong

ron96g · 2026-04-01T08:41:41Z

rover-server/internal/controller/roadmap.go

+
+		// Create resource ID
+		resourceId := roadmap.Namespace[len(security.PrefixFromContext(ctx)):] + "--" + roadmap.Name
+		if len(roadmap.Namespace) > 0 && roadmap.Namespace[0:len(security.PrefixFromContext(ctx))] != security.PrefixFromContext(ctx) {


ron96g · 2026-04-01T08:43:28Z

rover-server/internal/controller/roadmap.go

+			return nil, problems.InternalServerError("Failed to download roadmap items", err.Error())
+		}
+
+		b, err := io.ReadAll(reader)


Could use json.NewDecoder(...).Decode(...) instead

julius-malcovsky and others added 2 commits March 30, 2026 22:37

julius-malcovsky self-assigned this Mar 30, 2026

julius-malcovsky requested review from BjoernKarma, Copilot, iagarba, lukas016, ron96g and stefan-ctrl March 30, 2026 20:43

Copilot started reviewing on behalf of julius-malcovsky March 30, 2026 20:43 View session

Merge branch 'main' into feat/roadmap

a9b7c03

Copilot AI reviewed Mar 30, 2026

View reviewed changes

ron96g requested changes Apr 1, 2026

View reviewed changes

Conversation

julius-malcovsky commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

ron96g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants