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.

rover-server/internal/controller/roadmap.go

ron96g

Will review further. For now, some open questions.

docs/roadmap-feature.md

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?

maybe see answer under comment "If a Roadmap references an ApiSpecification, which does not exist. How should this be handled?"

im in between. both make sense ...

I just want to avoid adding a new way to reference resources, we often use explictly references via types.ObjectRef or types.TypedObjectRef but also implicitly via some internal convention. This now introduces a new way via ResourceType... Question is if it makes sense to use TypedObjectRef which (potentially) points to an object that does not exist yet. If we can accept that as a state then I would prefer that over ResourceType. ResourceName could then be called string Discriminator which either has the value of the basePath or the eventType.

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

I would keep the new ones, since they are more flexible. Perhaps we could add special endpoints for backwards compatibility. they would accept the old format and map it into the new one. and we can announce the deprecation of the old ones, have the new ones and support the migration :)

In theory, it would also be possible to do some sort of mapping in rover-ctl. E. g. if provided file is kind==ApiRoadmap, rover-ctl maps this to the new structure. However, the idea of rover-ctl is to "keep it dumb". so if we do this, it must be really minimal.

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?

I think a loose coupling is better here. It give you the possibility to provider a "future" roadmap for something youre just working on. Also, it would create an order dependency, or would add complexity with blocked statuses and watchers etc. I liked the idea of Roadmaps being lightweight and be used as non functional metadata. Plus they are optional.

What we could do is to return something to the user if they are referring to something that doesnt exist. Perhaps its intended and maybe its a typo and they would correct it.

Makes sense. It could be done using the priority ordering in rover-ctl, however, I agree that it should be an optional and lightweight documentation. There is the idea that we return warnings to Users, however, that has not been implemented yet and I think its out-of-scope. So lets keep it like it is right now

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) {


ooooops :)

resourceId is now handled properly - like ApiSpecification / EventSpecification

rover-server/internal/controller/roadmap.go

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>

ron96g · 2026-04-08T12:14:27Z

Both ApiRoadmap and ApiChanlog are missing the handler implementation in https://github.com/telekom/controlplane/blob/main/rover-ctl/pkg/handlers/registry.go#L55

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

rover-server/internal/controller/roadmap.go Show resolved Hide resolved

rover-server/internal/controller/roadmap.go Show resolved Hide resolved

rover-server/internal/controller/roadmap.go Outdated Show resolved Hide resolved

ron96g requested changes Apr 1, 2026

View reviewed changes

julius-malcovsky and others added 5 commits April 7, 2026 21:48

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!

Uh oh!

Uh oh!

Uh oh!

ron96g left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

Uh oh!

ron96g commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants