Skip to content

Commit 4bd9267

Browse files
enrico-kaack-compenrico-kaack-comp
authored
chore: ensure external-name compliance for KymaEnvironment (#542)
* implement external name convention * removed unused function * fix migration * fix wrong e2e and upgrade test with wrong configuration * pr review * fix landscape * add parameters for kymaenv
1 parent b120cd7 commit 4bd9267

22 files changed

Lines changed: 655 additions & 193 deletions

File tree

‎apis/environment/v1alpha1/kymaenvironment_types.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ type RetryStatus struct {
109109
// +kubebuilder:object:root=true
110110

111111
// A KymaEnvironment is a managed resource that represents a Kyma environment in the SAP Business Technology Platform
112+
//
113+
// External-Name Configuration:
114+
// - Follows Standard: yes
115+
// - Format: Environment Instance GUID (UUID format)
116+
// - How to find:
117+
// - UI: BTP Cockpit → Subaccounts → [Select Subaccount] → Instances and Subscriptions → Instance ID
118+
// - CLI: Use BTP ClI: `btp list accounts/environment-instance`
119+
//
112120
// +kubebuilder:printcolumn:name="READY",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
113121
// +kubebuilder:printcolumn:name="SYNCED",type="string",JSONPath=".status.conditions[?(@.type=='Synced')].status"
114122
// +kubebuilder:printcolumn:name="EXTERNAL-NAME",type="string",JSONPath=".metadata.annotations.crossplane\\.io/external-name"

‎btp/environment.go‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,6 @@ import (
88
provisioningclient "github.com/sap/crossplane-provider-btp/internal/openapi_clients/btp-provisioning-service-api-go/pkg"
99
)
1010

11-
// First tries to get the environment by id, if not found, it tries to get it by name and type
12-
func (c *Client) GetEnvironment(
13-
ctx context.Context, Id string, instanceName string, environmentType EnvironmentType,
14-
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, error) {
15-
// Try to get the environment by id first
16-
environmentInstance, err := c.GetEnvironmentById(ctx, Id)
17-
if err != nil {
18-
return nil, err
19-
}
20-
21-
if environmentInstance != nil {
22-
return environmentInstance, nil
23-
}
24-
25-
// If not found by id, try to get it by name and type
26-
return c.GetEnvironmentByNameAndType(ctx, instanceName, environmentType)
27-
}
28-
2911
func (c *Client) GetEnvironmentInstanceByID(ctx context.Context, instanceID string) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, bool, error) {
3012
response, resp, err := c.ProvisioningServiceClient.GetEnvironmentInstance(ctx, instanceID).Execute()
3113

@@ -36,45 +18,6 @@ func (c *Client) GetEnvironmentInstanceByID(ctx context.Context, instanceID stri
3618
return response, false, nil
3719
}
3820

39-
// GetEnvironmentByNameAndType retrieves environment using its name and type. It performs a list and filters client-side.
40-
// Deprecated: use GetEnvironmentsByID instead.
41-
func (c *Client) GetEnvironmentByNameAndType(
42-
ctx context.Context, instanceName string, environmentType EnvironmentType,
43-
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, error) {
44-
var environmentInstance *provisioningclient.BusinessEnvironmentInstanceResponseObject
45-
// additional Authorization param needs to be set != nil to avoid client blocking the call due to mandatory condition in specs
46-
response, _, err := c.ProvisioningServiceClient.GetEnvironmentInstances(ctx).Authorization("").Execute()
47-
48-
if err != nil {
49-
return nil, specifyAPIError(err)
50-
}
51-
52-
for _, instance := range response.EnvironmentInstances {
53-
if instance.EnvironmentType != nil && *instance.EnvironmentType != environmentType.Identifier {
54-
continue
55-
}
56-
57-
var parameters string
58-
var parameterList map[string]interface{}
59-
if instance.Parameters != nil {
60-
parameters = *instance.Parameters
61-
}
62-
err := json.Unmarshal([]byte(parameters), &parameterList)
63-
if err != nil {
64-
return nil, err
65-
}
66-
if parameterList[cfenvironmentParameterInstanceName] == instanceName {
67-
environmentInstance = &instance
68-
break
69-
}
70-
if parameterList[KymaenvironmentParameterInstanceName] == instanceName {
71-
environmentInstance = &instance
72-
break
73-
}
74-
}
75-
return environmentInstance, err
76-
}
77-
7821
// GetEnvironmentById retrieves environment using its ID. It performs a list and filters client-side.
7922
// Deprecated: use GetEnvironmentInstanceByID instead.
8023
func (c *Client) GetEnvironmentById(

‎btp/kyma_environment.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package btp
22

33
import (
44
"context"
5+
"encoding/json"
56

67
"github.com/sap/crossplane-provider-btp/internal"
78
provisioningclient "github.com/sap/crossplane-provider-btp/internal/openapi_clients/btp-provisioning-service-api-go/pkg"
@@ -14,6 +15,31 @@ func KymaEnvironmentType() EnvironmentType {
1415
}
1516
}
1617

18+
// GetKymaEnvironment retrieves a Kyma environment using either UUID-based lookup (new format, >= v1.2.2)
19+
// or name-based lookup (legacy format, < v1.2.2).
20+
//
21+
// For UUID external-names: directly retrieves by ID, returns nil if not found (drift scenario).
22+
// For legacy name external-names: falls back to GetEnvironmentByNameAndType which lists and filters client-side.
23+
func (c *Client) GetKymaEnvironment(
24+
ctx context.Context, Id string, instanceName string, environmentType EnvironmentType,
25+
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, error) {
26+
if internal.IsValidUUID(Id) {
27+
// New format (>= v1.2.2): external-name is a UUID, use direct ID lookup
28+
environmentInstance, notFound, err := c.GetEnvironmentInstanceByID(ctx, Id)
29+
if notFound {
30+
// 404 is not an error - resource doesn't exist (drift scenario)
31+
return nil, nil
32+
}
33+
if err != nil {
34+
return nil, err
35+
}
36+
return environmentInstance, nil
37+
}
38+
39+
// Legacy format (< v1.2.2): external-name is the instance name, use name-based lookup
40+
return c.GetEnvironmentByNameAndType(ctx, instanceName, environmentType)
41+
}
42+
1743
func (c *Client) CreateKymaEnvironment(ctx context.Context, instanceName string, planeName string, parameters InstanceParameters, resourceUID string, serviceAccountEmail string) (string, error) {
1844
envType := KymaEnvironmentType()
1945
payload := provisioningclient.CreateEnvironmentInstanceRequestPayload{
@@ -49,3 +75,43 @@ func (c *Client) UpdateKymaEnvironment(ctx context.Context, environmentInstanceI
4975

5076
return nil
5177
}
78+
79+
// GetEnvironmentByNameAndType retrieves environment using its name and type. It performs a list and filters client-side.
80+
// Deprecated: use GetEnvironmentsByID instead.
81+
func (c *Client) GetEnvironmentByNameAndType(
82+
ctx context.Context, instanceName string, environmentType EnvironmentType,
83+
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, error) {
84+
var environmentInstance *provisioningclient.BusinessEnvironmentInstanceResponseObject
85+
// additional Authorization param needs to be set != nil to avoid client blocking the call due to mandatory condition in specs
86+
response, _, err := c.ProvisioningServiceClient.GetEnvironmentInstances(ctx).Authorization("").Execute()
87+
88+
if err != nil {
89+
return nil, specifyAPIError(err)
90+
}
91+
92+
for _, instance := range response.EnvironmentInstances {
93+
if instance.EnvironmentType != nil && *instance.EnvironmentType != environmentType.Identifier {
94+
continue
95+
}
96+
97+
var parameters string
98+
var parameterList map[string]interface{}
99+
if instance.Parameters != nil {
100+
parameters = *instance.Parameters
101+
}
102+
err := json.Unmarshal([]byte(parameters), &parameterList)
103+
if err != nil {
104+
return nil, err
105+
}
106+
// keeping the old parameter to not potentially break systems, function is deprecated and should not be used anymore
107+
if parameterList[cfenvironmentParameterInstanceName] == instanceName {
108+
environmentInstance = &instance
109+
break
110+
}
111+
if parameterList[KymaenvironmentParameterInstanceName] == instanceName {
112+
environmentInstance = &instance
113+
break
114+
}
115+
}
116+
return environmentInstance, err
117+
}

‎docs/user/external-name.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ metadata.annotations.crossplane.io/external-name: <resource_uniq_ID>
5151
- Follows Standard: no - This resource does not support external name as it does not represent an external resource. Instead of using external name for importing, you can just create an instance of this resource.
5252
- Format: Not applicable
5353

54+
### KymaEnvironment
55+
56+
- Follows Standard: yes
57+
- Format: Environment Instance GUID (UUID format)
58+
- How to find:
59+
60+
- UI: BTP Cockpit → Subaccounts → [Select Subaccount] → Instances and Subscriptions → Instance ID
61+
- CLI: Use BTP ClI: `btp list accounts/environment-instance`
62+
5463
### Subaccount
5564

5665
- Follows Standard: yes

‎internal/clients/kymaenvironment/client.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@ import (
1919
type Client interface {
2020
DescribeInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) (
2121
*provisioningclient.BusinessEnvironmentInstanceResponseObject,
22-
bool, // true if the external name was updated
2322
error,
2423
)
2524
CreateInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) (string, error)
2625
UpdateInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) error
27-
DeleteInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) error
26+
// DeleteInstance deletes the Kyma environment using the external-name.
27+
// Returns the HTTP response (for status code checking) and any error.
28+
DeleteInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) (*http.Response, error)
2829
}
2930

3031
func GenerateObservation(

‎internal/clients/kymaenvironment/kymaenvironment.go‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package environments
22

33
import (
44
"context"
5+
"net/http"
56

67
"github.com/crossplane/crossplane-runtime/pkg/errors"
78
"github.com/crossplane/crossplane-runtime/pkg/meta"
@@ -16,7 +17,7 @@ import (
1617
const (
1718
errKymaInstanceCreateFailed = "Could not create KymaEnvironment"
1819
errKymaInstanceUpdateFailed = "Could not update KymaEnvironment"
19-
errInstanceIdNotFound = "Could not update kyma instance .status.AtProvider.Id is empty"
20+
errExternalNameNotFound = "external-name not set"
2021
)
2122

2223
type KymaEnvironments struct {
@@ -27,28 +28,31 @@ func NewKymaEnvironments(btp btp.Client) *KymaEnvironments {
2728
return &KymaEnvironments{btp: btp}
2829
}
2930

31+
// DescribeInstance retrieves a Kyma environment instance using the external-name annotation.
32+
// Supports both UUID-based (>= v1.2.2) and name-based (< v1.2.2) external-name formats.
33+
// Returns nil if the instance doesn't exist (without error).
3034
func (c KymaEnvironments) DescribeInstance(
3135
ctx context.Context,
3236
cr v1alpha1.KymaEnvironment,
33-
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, bool, error) {
37+
) (*provisioningclient.BusinessEnvironmentInstanceResponseObject, error) {
38+
// If external-name is empty, resource needs to be created. Should be checked by the caller already
39+
externalName := meta.GetExternalName(&cr)
40+
if externalName == "" {
41+
return nil, nil
42+
}
3443

35-
environment, err := c.btp.GetEnvironment(ctx, meta.GetExternalName(&cr), GetKymaEnvironmentName(cr), btp.KymaEnvironmentType())
44+
// GetKymaEnvironment handles both UUID-based and legacy name-based lookups
45+
environment, err := c.btp.GetKymaEnvironment(ctx, externalName, GetKymaEnvironmentName(cr), btp.KymaEnvironmentType())
3646

3747
if err != nil {
38-
return nil, false, err
48+
return nil, err
3949
}
4050

4151
if environment == nil {
42-
return nil, false, nil
43-
}
44-
45-
// If the external name is not set yet, we set it to the ID of the environment. And force an update.
46-
if *environment.Id != meta.GetExternalName(&cr) {
47-
meta.SetExternalName(&cr, *environment.Id)
48-
return environment, true, nil
52+
return nil, nil
4953
}
5054

51-
return environment, false, nil
55+
return environment, nil
5256
}
5357

5458
func (c KymaEnvironments) CreateInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) (string, error) {
@@ -72,18 +76,24 @@ func (c KymaEnvironments) CreateInstance(ctx context.Context, cr v1alpha1.KymaEn
7276
return guid, nil
7377
}
7478

75-
func (c KymaEnvironments) DeleteInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) error {
76-
if cr.Status.AtProvider.ID == nil {
77-
return errors.New(errInstanceIdNotFound)
79+
// DeleteInstance deletes the Kyma environment using the external-name (GUID).
80+
// Returns the HTTP response for status code checking and any error.
81+
func (c KymaEnvironments) DeleteInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) (*http.Response, error) {
82+
externalName := meta.GetExternalName(&cr)
83+
84+
// Use external-name (GUID) for deletion
85+
if externalName == "" {
86+
return nil, errors.New(errExternalNameNotFound)
7887
}
79-
_, err := c.btp.DeleteEnvironmentInstanceByID(ctx, *cr.Status.AtProvider.ID)
80-
return err
88+
89+
return c.btp.DeleteEnvironmentInstanceByID(ctx, externalName)
8190
}
8291

8392
func (c KymaEnvironments) UpdateInstance(ctx context.Context, cr v1alpha1.KymaEnvironment) error {
93+
externalName := meta.GetExternalName(&cr)
8494

85-
if cr.Status.AtProvider.ID == nil {
86-
return errors.New(errInstanceIdNotFound)
95+
if externalName == "" {
96+
return errors.New(errExternalNameNotFound)
8797
}
8898

8999
parameters, err := internal.UnmarshalRawParameters(cr.Spec.ForProvider.Parameters.Raw)
@@ -93,7 +103,7 @@ func (c KymaEnvironments) UpdateInstance(ctx context.Context, cr v1alpha1.KymaEn
93103
}
94104
err = c.btp.UpdateKymaEnvironment(
95105
ctx,
96-
*cr.Status.AtProvider.ID,
106+
externalName,
97107
cr.Spec.ForProvider.PlanName,
98108
parameters,
99109
string(cr.UID),

0 commit comments

Comments
 (0)