Fix: Portal asset and api publication redundant update plans (#323)

* Fix: adds portal asset checking to planner

* Fix: updates assets e2e test for fix to asset file comparsion fix

* Fix: updates assets e2e test for fix to asset file comparsion fix

* Fix: Ensures api publications are idempotent when auth strat ids are present

---------

Co-authored-by: Rick Spurgeon <10521262+rspurgeon@users.noreply.github.com>

Author: rspurgeon

.go-version

@@ -0,0 +1 @@
+1.24.3

.tool-versions

@@ -0,0 +1,3 @@
+jq 1.8.1
+gh 2.83.2
+git 2.52.0

docs/examples/declarative/basic/api-with-portal-pub.yaml

@@ -1,9 +1,22 @@
+application_auth_strategies:
+  - ref: simple-auth
+    name: my-api-key-auth
+    display_name: API Key Authentication
+    strategy_type: key_auth
+    configs:
+      key_auth:
+        key_names:
+          - X-API-Key
 
 portals:
   - ref: simple-portal
     name: My Simple Portal 
     display_name: Simple Portal ©
     description: The simplest Portal example 
+    # Portal assets (logo and favicon)
+    assets:
+      logo: !file ../portal/assets/logo.png
+      favicon: !file ../portal/assets/favicon.png
 
 apis:
   - ref: simple-api
@@ -13,3 +26,5 @@ apis:
       - ref: simple-api-to-simple-portal 
         portal_id: !ref simple-portal#id
         visibility: public
+        auth_strategy_ids:
+          - !ref simple-auth#id 

internal/declarative/planner/api_planner.go

@@ -1266,8 +1266,11 @@ func (p *Planner) shouldUpdateAPIPublication(
 	updates := make(map[string]any)
 
 	// Compare auth strategy IDs (order-independent comparison)
-	if !p.compareStringSlices(current.AuthStrategyIDs, desired.AuthStrategyIds) {
-		updates["auth_strategy_ids"] = desired.AuthStrategyIds
+	if desired.AuthStrategyIds != nil {
+		resolvedDesired := p.resolveAuthStrategyIDsForComparison(desired.AuthStrategyIds)
+		if !p.compareStringSlices(current.AuthStrategyIDs, resolvedDesired) {
+			updates["auth_strategy_ids"] = desired.AuthStrategyIds
+		}
 	}
 
 	// Compare auto approve registrations
@@ -1290,6 +1293,33 @@ func (p *Planner) shouldUpdateAPIPublication(
 	return len(updates) > 0, updates
 }
 
+func (p *Planner) resolveAuthStrategyIDsForComparison(desired []string) []string {
+	if p.resources == nil || len(desired) == 0 {
+		return desired
+	}
+
+	resolved := make([]string, 0, len(desired))
+	for _, ref := range desired {
+		lookupRef := ref
+		if tags.IsRefPlaceholder(lookupRef) {
+			if parsedRef, _, ok := tags.ParseRefPlaceholder(lookupRef); ok && parsedRef != "" {
+				lookupRef = parsedRef
+			}
+		}
+
+		if strategy := p.resources.GetAuthStrategyByRef(lookupRef); strategy != nil {
+			if id := strategy.GetKonnectID(); id != "" {
+				resolved = append(resolved, id)
+				continue
+			}
+		}
+
+		resolved = append(resolved, ref)
+	}
+
+	return resolved
+}
+
 // compareStringSlices compares two string slices for equality (order-independent)
 func (p *Planner) compareStringSlices(a, b []string) bool {
 	if len(a) != len(b) {

internal/declarative/planner/api_planner_test.go

@@ -6,7 +6,9 @@ import (
 	kkComps "github.com/Kong/sdk-konnect-go/models/components"
 	"github.com/kong/kongctl/internal/declarative/resources"
 	"github.com/kong/kongctl/internal/declarative/state"
+	"github.com/kong/kongctl/internal/declarative/tags"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestAPIVersionConstraintValidation tests that the loader properly validates API version constraints
@@ -96,3 +98,64 @@ func TestShouldUpdateAPIConsidersSlugAndAttributes(t *testing.T) {
 	assert.Equal(t, updatedSlug, updateFields["slug"])
 	assert.Equal(t, expectedUpdatedAttrs, updateFields["attributes"])
 }
+
+func TestShouldUpdateAPIPublicationResolvesAuthStrategyRefs(t *testing.T) {
+	t.Parallel()
+
+	authStrategy := resources.ApplicationAuthStrategyResource{
+		CreateAppAuthStrategyRequest: kkComps.CreateCreateAppAuthStrategyRequestKeyAuth(
+			kkComps.AppAuthStrategyKeyAuthRequest{
+				Name:         "my-api-key-auth",
+				StrategyType: kkComps.StrategyTypeKeyAuth,
+			},
+		),
+		Ref: "key-auth",
+	}
+
+	authStrategy.TryMatchKonnectResource(state.ApplicationAuthStrategy{
+		ID:   "auth-id",
+		Name: "my-api-key-auth",
+	})
+
+	planner := &Planner{
+		resources: &resources.ResourceSet{
+			ApplicationAuthStrategies: []resources.ApplicationAuthStrategyResource{authStrategy},
+		},
+	}
+
+	current := state.APIPublication{
+		AuthStrategyIDs: []string{"auth-id"},
+	}
+
+	desired := resources.APIPublicationResource{
+		APIPublication: kkComps.APIPublication{
+			AuthStrategyIds: []string{tags.RefPlaceholderPrefix + "key-auth#id"},
+		},
+		Ref:      "pub",
+		PortalID: "portal-id",
+	}
+
+	needsUpdate, fields := planner.shouldUpdateAPIPublication(current, desired)
+	require.False(t, needsUpdate)
+	assert.Empty(t, fields)
+}
+
+func TestShouldUpdateAPIPublicationIgnoresAuthStrategyWhenUnset(t *testing.T) {
+	t.Parallel()
+
+	planner := &Planner{}
+
+	current := state.APIPublication{
+		AuthStrategyIDs: []string{"auth-id"},
+	}
+
+	desired := resources.APIPublicationResource{
+		APIPublication: kkComps.APIPublication{},
+		Ref:            "pub",
+		PortalID:       "portal-id",
+	}
+
+	needsUpdate, fields := planner.shouldUpdateAPIPublication(current, desired)
+	require.False(t, needsUpdate)
+	assert.Empty(t, fields)
+}

internal/declarative/planner/portal_asset_test.go

@@ -0,0 +1,87 @@
+package planner
+
+import (
+	"context"
+	"encoding/base64"
+	"fmt"
+	"net/http"
+	"testing"
+
+	kkErrors "github.com/Kong/sdk-konnect-go/models/sdkerrors"
+	"github.com/stretchr/testify/require"
+)
+
+func makeDataURL(mime string, payload []byte) string {
+	encoded := base64.StdEncoding.EncodeToString(payload)
+	return fmt.Sprintf("data:%s;base64,%s", mime, encoded)
+}
+
+func TestDataURLsEqual_Base64(t *testing.T) {
+	t.Parallel()
+
+	payload := []byte("hello")
+	first := makeDataURL("image/png", payload)
+	second := makeDataURL("image/jpeg", payload)
+
+	equal, err := dataURLsEqual(first, second)
+	require.NoError(t, err)
+	require.True(t, equal)
+}
+
+func TestPortalAssetNeedsUpdate_NotFound(t *testing.T) {
+	t.Parallel()
+
+	desired := makeDataURL("image/png", []byte("asset"))
+	planner := &Planner{}
+
+	needsUpdate, err := planner.portalAssetNeedsUpdate(
+		context.Background(),
+		"portal-id",
+		desired,
+		func(_ context.Context, _ string) (string, error) {
+			return "", &kkErrors.SDKError{StatusCode: http.StatusNotFound}
+		},
+	)
+
+	require.NoError(t, err)
+	require.True(t, needsUpdate)
+}
+
+func TestPortalAssetNeedsUpdate_NoChange(t *testing.T) {
+	t.Parallel()
+
+	payload := []byte("asset")
+	desired := makeDataURL("image/png", payload)
+	planner := &Planner{}
+
+	needsUpdate, err := planner.portalAssetNeedsUpdate(
+		context.Background(),
+		"portal-id",
+		desired,
+		func(_ context.Context, _ string) (string, error) {
+			return makeDataURL("image/jpeg", payload), nil
+		},
+	)
+
+	require.NoError(t, err)
+	require.False(t, needsUpdate)
+}
+
+func TestPortalAssetNeedsUpdate_Change(t *testing.T) {
+	t.Parallel()
+
+	desired := makeDataURL("image/png", []byte("asset"))
+	planner := &Planner{}
+
+	needsUpdate, err := planner.portalAssetNeedsUpdate(
+		context.Background(),
+		"portal-id",
+		desired,
+		func(_ context.Context, _ string) (string, error) {
+			return makeDataURL("image/png", []byte("different")), nil
+		},
+	)
+
+	require.NoError(t, err)
+	require.True(t, needsUpdate)
+}