Implement RegisterEntity consistently across providers, with test (#6047)

* Implement RegisterEntity consistently across providers, with test

* Fix lint errors

Author: evankanderson

internal/providers/dockerhub/dockerhub.go

@@ -194,11 +194,14 @@ func (*dockerHubImageLister) SupportsEntity(_ minderv1.Entity) bool {
 }
 
 // RegisterEntity implements the Provider interface
-func (*dockerHubImageLister) RegisterEntity(
-	_ context.Context, _ minderv1.Entity, _ *properties.Properties,
+func (d *dockerHubImageLister) RegisterEntity(
+	_ context.Context, entType minderv1.Entity, props *properties.Properties,
 ) (*properties.Properties, error) {
-	// TODO: implement
-	return nil, nil
+	if !d.SupportsEntity(entType) {
+		return nil, provifv1.ErrUnsupportedEntity
+	}
+	// we don't need to do any explicit registration
+	return props, nil
 }
 
 // DeregisterEntity implements the Provider interface
@@ -209,14 +212,6 @@ func (*dockerHubImageLister) DeregisterEntity(
 	return nil
 }
 
-// ReregisterEntity implements the Provider interface
-func (*dockerHubImageLister) ReregisterEntity(
-	_ context.Context, _ minderv1.Entity, _ *properties.Properties,
-) error {
-	// TODO: implement
-	return nil
-}
-
 // PropertiesToProtoMessage implements the Provider interface
 func (*dockerHubImageLister) PropertiesToProtoMessage(
 	_ minderv1.Entity, _ *properties.Properties) (protoreflect.ProtoMessage, error) {

internal/providers/dockerhub/provider_test.go

@@ -0,0 +1,17 @@
+// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package dockerhub
+
+import (
+	"testing"
+
+	testhelper "github.com/mindersec/minder/pkg/providers/v1/testing"
+)
+
+func TestRegistration(t *testing.T) {
+	t.Parallel()
+	// We don't need a full constructor here, so we're naughty
+	dh := &dockerHubImageLister{}
+	testhelper.CheckRegistrationExcept(t, dh)
+}

internal/providers/github/entities.go

@@ -19,6 +19,7 @@ import (
 	"github.com/mindersec/minder/internal/util/ptr"
 	minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
 	"github.com/mindersec/minder/pkg/entities/properties"
+	provifv1 "github.com/mindersec/minder/pkg/providers/v1"
 )
 
 var (
@@ -49,12 +50,28 @@ func (c *GitHub) SupportsEntity(entType minderv1.Entity) bool {
 func (c *GitHub) RegisterEntity(
 	ctx context.Context, entityType minderv1.Entity, props *properties.Properties,
 ) (*properties.Properties, error) {
-	// We only need explicit registration steps for repositories
-	// The rest of the entities are originated from them.
-	if entityType != minderv1.Entity_ENTITY_REPOSITORIES {
+	if !c.SupportsEntity(entityType) {
+		return nil, provifv1.ErrUnsupportedEntity
+	}
+
+	//nolint:exhaustive
+	switch entityType {
+	case minderv1.Entity_ENTITY_PULL_REQUESTS:
+		fallthrough
+	case minderv1.Entity_ENTITY_ARTIFACTS:
+		fallthrough
+	case minderv1.Entity_ENTITY_RELEASE:
+		// Nothing to do, accept:
 		return props, nil
+	case minderv1.Entity_ENTITY_REPOSITORIES:
+		return c.registerRepoWebhook(ctx, props)
+	default:
+		return nil, provifv1.ErrUnsupportedEntity
 	}
+}
 
+func (c *GitHub) registerRepoWebhook(ctx context.Context, props *properties.Properties,
+) (*properties.Properties, error) {
 	webhookURLProvider, err := url.JoinPath(
 		c.webhookConfig.ExternalWebhookURL,
 		url.PathEscape(string(db.ProviderTypeGithub)),
@@ -147,57 +164,6 @@ func (c *GitHub) DeregisterEntity(ctx context.Context, entityType minderv1.Entit
 	return nil
 }
 
-// ReregisterEntity implements the Provider interface
-func (c *GitHub) ReregisterEntity(
-	ctx context.Context, entityType minderv1.Entity, props *properties.Properties,
-) error {
-	// We only need explicit registration steps for repositories
-	// The rest of the entities are originated from them.
-	if entityType != minderv1.Entity_ENTITY_REPOSITORIES {
-		return nil
-	}
-
-	repoNameP := props.GetProperty(ghprop.RepoPropertyName)
-	if repoNameP == nil {
-		return errors.New("repo name property not found")
-	}
-
-	repoOwnerP := props.GetProperty(ghprop.RepoPropertyOwner)
-	if repoOwnerP == nil {
-		return errors.New("repo owner property not found")
-	}
-
-	hookIDP := props.GetProperty(ghprop.RepoPropertyHookId)
-	if hookIDP == nil {
-		return errors.New("hook ID property not found")
-	}
-
-	hookURL := props.GetProperty(ghprop.RepoPropertyHookUrl)
-	if hookURL == nil {
-		return errors.New("hook URL property not found")
-	}
-
-	ping := c.webhookConfig.ExternalPingURL
-
-	secret, err := c.webhookConfig.GetWebhookSecret()
-	if err != nil {
-		return fmt.Errorf("error getting webhook secret for github provider: %w", err)
-	}
-
-	repoName := repoNameP.GetString()
-	repoOwner := repoOwnerP.GetString()
-	hookID := hookIDP.GetInt64()
-
-	hook := getGitHubWebhook(hookURL.GetString(), ping, secret)
-	_, err = c.EditHook(ctx, repoOwner, repoName, hookID, hook)
-	if err != nil {
-		zerolog.Ctx(ctx).Error().Err(err).Msg("unable to update hook")
-		return fmt.Errorf("unable to update hook: %w", err)
-	}
-
-	return nil
-}
-
 func (c *GitHub) cleanupStaleHooks(
 	ctx context.Context,
 	repoOwner string,

internal/providers/github/ghcr/ghcr.go

@@ -155,11 +155,14 @@ func (*ImageLister) SupportsEntity(_ minderv1.Entity) bool {
 }
 
 // RegisterEntity implements the Provider interface
-func (*ImageLister) RegisterEntity(
-	_ context.Context, _ minderv1.Entity, _ *properties.Properties,
+func (i *ImageLister) RegisterEntity(
+	_ context.Context, entType minderv1.Entity, props *properties.Properties,
 ) (*properties.Properties, error) {
-	// TODO: implement
-	return nil, nil
+	if !i.SupportsEntity(entType) {
+		return nil, provifv1.ErrUnsupportedEntity
+	}
+	// we don't need to do any explicit registration
+	return props, nil
 }
 
 // DeregisterEntity implements the Provider interface
@@ -168,13 +171,6 @@ func (*ImageLister) DeregisterEntity(_ context.Context, _ minderv1.Entity, _ *pr
 	return nil
 }
 
-// ReregisterEntity implements the Provider interface
-func (*ImageLister) ReregisterEntity(
-	_ context.Context, _ minderv1.Entity, _ *properties.Properties,
-) error {
-	return nil
-}
-
 // PropertiesToProtoMessage implements the Provider interface
 func (*ImageLister) PropertiesToProtoMessage(_ minderv1.Entity, _ *properties.Properties) (protoreflect.ProtoMessage, error) {
 	// TODO: Implement

internal/providers/github/ghcr/provider_test.go

@@ -0,0 +1,17 @@
+// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package ghcr
+
+import (
+	"testing"
+
+	testhelper "github.com/mindersec/minder/pkg/providers/v1/testing"
+)
+
+func TestRegistration(t *testing.T) {
+	t.Parallel()
+	// We don't need a full constructor here, so we're naughty
+	il := &ImageLister{}
+	testhelper.CheckRegistrationExcept(t, il)
+}

internal/providers/github/provider_test.go

@@ -0,0 +1,23 @@
+// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package github
+
+import (
+	"testing"
+
+	"github.com/mindersec/minder/internal/providers/github/properties"
+	minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
+	testhelper "github.com/mindersec/minder/pkg/providers/v1/testing"
+)
+
+func TestRegistration(t *testing.T) {
+	t.Parallel()
+	// We don't need a full constructor here, so we're naughty
+	gh := &GitHub{
+		propertyFetchers: properties.NewPropertyFetcherFactory(),
+	}
+	// Repositories do a bunch of special registration, so skip them
+	// in this test -- we test them in common_test.go.
+	testhelper.CheckRegistrationExcept(t, gh, minderv1.Entity_ENTITY_REPOSITORIES)
+}

internal/providers/gitlab/registration.go

@@ -20,14 +20,15 @@ import (
 	"github.com/mindersec/minder/internal/util/ptr"
 	minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
 	"github.com/mindersec/minder/pkg/entities/properties"
+	provifv1 "github.com/mindersec/minder/pkg/providers/v1"
 )
 
 // RegisterEntity implements the Provider interface
 func (c *gitlabClient) RegisterEntity(
 	ctx context.Context, entType minderv1.Entity, props *properties.Properties,
 ) (*properties.Properties, error) {
 	if !c.SupportsEntity(entType) {
-		return nil, errors.New("unsupported entity type")
+		return nil, provifv1.ErrUnsupportedEntity
 	}
 
 	if entType != minderv1.Entity_ENTITY_REPOSITORIES {
@@ -88,32 +89,6 @@ func (c *gitlabClient) DeregisterEntity(
 	return nil
 }
 
-// ReregisterEntity implements the Provider interface
-func (c *gitlabClient) ReregisterEntity(
-	ctx context.Context, entType minderv1.Entity, props *properties.Properties,
-) error {
-	if !c.SupportsEntity(entType) {
-		return errors.New("unsupported entity type")
-	}
-
-	upstreamID := props.GetProperty(properties.PropertyUpstreamID).GetString()
-	if upstreamID == "" {
-		return errors.New("missing upstream ID")
-	}
-
-	hookID := props.GetProperty(RepoPropertyHookID).GetString()
-	if hookID == "" {
-		return errors.New("missing hook ID")
-	}
-
-	hookURL := props.GetProperty(RepoPropertyHookURL).GetString()
-	if hookURL == "" {
-		return errors.New("missing hook URL")
-	}
-
-	return c.updateWebhook(ctx, upstreamID, hookID, hookURL)
-}
-
 func (c *gitlabClient) createWebhook(ctx context.Context, upstreamID string) (*properties.Properties, error) {
 	createHookPath, err := url.JoinPath("projects", upstreamID, "hooks")
 	if err != nil {
@@ -236,64 +211,3 @@ func (c *gitlabClient) cleanUpStaleWebhooks(ctx context.Context, upstreamID stri
 
 	return nil
 }
-
-func (c *gitlabClient) updateWebhook(ctx context.Context, upstreamID, hookID string, hookURL string) error {
-	// We don't need to update the webhook URL, as it's unique for each
-	// registration. We only need to update the secret.
-	updateHookPath, err := url.JoinPath("projects", upstreamID, "hooks", hookID)
-	if err != nil {
-		return fmt.Errorf("failed to join URL path for hook: %w", err)
-	}
-
-	hookURLParsed, err := url.Parse(hookURL)
-	if err != nil {
-		return fmt.Errorf("failed to parse hook URL: %w", err)
-	}
-
-	// We need to extract the UUID from the webhook URL. The UUID is
-	// the last part of the path.
-	hookMinderUUID := hookURLParsed.Path[strings.LastIndex(hookURLParsed.Path, "/")+1:]
-
-	sec, err := webhooksecret.New(c.currentWebhookSecret, hookMinderUUID)
-	if err != nil {
-		return fmt.Errorf("failed to create webhook secret: %w", err)
-	}
-
-	trve := ptr.Ptr(true)
-	hreq := &gitlab.EditProjectHookOptions{
-		URL:                   &hookURL,
-		Token:                 &sec,
-		PushEvents:            trve,
-		TagPushEvents:         trve,
-		MergeRequestsEvents:   trve,
-		ReleasesEvents:        trve,
-		EnableSSLVerification: trve,
-	}
-
-	if err := c.doUpdateWebhook(ctx, updateHookPath, hreq); err != nil {
-		return fmt.Errorf("failed to update webhook: %w", err)
-	}
-
-	return nil
-}
-
-func (c *gitlabClient) doUpdateWebhook(
-	ctx context.Context, updateHookPath string, hreq *gitlab.EditProjectHookOptions,
-) error {
-	req, err := c.NewRequest(http.MethodPut, updateHookPath, hreq)
-	if err != nil {
-		return fmt.Errorf("failed to create request: %w", err)
-	}
-
-	resp, err := c.Do(ctx, req)
-	if err != nil {
-		return fmt.Errorf("failed to get projects: %w", err)
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("unexpected status code: %d", resp.StatusCode)
-	}
-
-	return nil
-}