Add Component Feature Advertisement

[kiosion]

Context

Implements the initial phase of RFD 230 by adding a versioned ComponentFeatures payload to service presence, allowing consumers to reason about capabilities without relying on semver. For now, App Service, Auth, and Proxy advertise support for Resource Constraints in certificates (RFD 228), and are used by Proxy to set SupportedFeatureIDs on App records from clusterUnifiedResourcesGet.

Related

• See RFD 230 and RFD 228.
• Requires Add paginated ListAuthServers/ListProxies #61702.
• Required for Access Requests: Add Resource-Scoped Constraints #60123.

[smallinsky]

I did first quick pass. Left few questions suggestion.

Also the Test coverage like integrant test covering feature advertising from App Agent to the Unified Rescues view result seems to be missing.

[smallinsky]

Why we need to have Proxies Capabilities where we are handling the case types.AppServer: case ?

[smallinsky]

The aggregatedAppServer logic could be simplified and made consistent with the aggregatedKube approach

Test coverage is also missing for the “many app servers -> same app” scenario, and I think the flow should distinguish the proxy app feature from the app server feature more explicitly.

That said, aside from this and a few nit comments looks solid. LGTM once the remaining comments are addressed.

[smallinsky]

Why this method are needed ?

I fee like the aggregatedAppServer should overwrite the GetFeatures instead of trying to update on copy.

For instance you can take a look at aggregatedKube or agggragateDB where GetTargetHealthStatus method is overriten by wrapper

We don't want to introduce seconds solution solving the same problem but in different way.

[kiosion]

Copy()/CloneResource() are needed as unified cache returns clones so the intersection needs to be set on the cloned Resource otherwise the aggregation is lost when we copy/unwrap.

But I agree it'd be better to match the aggregatedKube/aggregateDatabase pattern. I’ll also override GetComponentFeatures() on the wrapper like GetTargetHealthStatus so Copy() can set ComponentFeatures from that method so both pre-copy and post-copy behaviour is consistent.

[smallinsky]

Copy()/CloneResource() are needed as unified cache returns clones so the intersection needs to be set on the cloned Resource otherwise the aggregation is lost when we copy/unwrap.

Why the aggregatedKube/aggregateDatabase don't need the Clone ? I think that we follow existing pattern this can be simplified.

[kiosion]

They do have copy/clone funcs?

teleport/lib/services/unified_resource.go

Lines 1112 to 1122 in 1fec131

	func (d *aggregatedDatabase) Copy() types.DatabaseServer {
	out := d.DatabaseServer.Copy()
	out.SetTargetHealthStatus(d.status)
	return out
	}
	// CloneResource returns a copy of the underlying database server with
	// aggregated health status.
	func (d *aggregatedDatabase) CloneResource() types.ResourceWithLabels {
	return d.Copy()
	}

[smallinsky]

LGTM when remaining comment will be addressed

[backport-bot-workflows]

@kiosion See the table below for backport results.

Branch	Result
branch/v18	Failed