search

crossplane / upjet

A code generation framework and runtime for Crossplane providers
Apache License 2.0
296 stars 86 forks source link

aws_glue_connection generation issues #126

Closed stevendborrelli closed 1 year ago

stevendborrelli commented 1 year ago

What happened?

aws_glue_connection (see code) is marked in the skip list due to terrajet issue #100, which is marked as being closed. I'm not sure if it is fixed in upjet.

When trying to use upjet to generate the APIs for this resource, we get the following issue 

panic: cannot run goimports for apis folder: ./glue/v1beta1/zz_connection_types.go:44:53: expected ';', found '/'
./glue/v1beta1/zz_connection_types.go:73:1: expected '}', found 'type'
: exit status 2

It looks like line 44 is causing the issue:

ConnectionPropertiesSecretRef *map[string]github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector%!(EXTRA string=v1) `json:"connectionPropertiesSecretRef,omitempty" tf:"-"`

The generated go code for v1beta1/zz_connection_types.go is:

/*
Copyright 2022 Upbound Inc.
*/

// Code generated by upjet. DO NOT EDIT.

package v1beta1

import (
    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
    "k8s.io/apimachinery/pkg/runtime/schema"

    v1 "mapstringgithub.com/crossplane/crossplane-runtime/apis/common/v1"
v1common "github.com/crossplane/crossplane-runtime/apis/common/v1"

)

type ConnectionObservation struct {

// The ARN of the Glue Connection.
Arn *string `json:"arn,omitempty" tf:"arn,omitempty"`

// Catalog ID and name of the connection
ID *string `json:"id,omitempty" tf:"id,omitempty"`

// A map of tags assigned to the resource, including those inherited from the provider default_tags configuration block.
TagsAll map[string]*string `json:"tagsAll,omitempty" tf:"tags_all,omitempty"`
}

type ConnectionParameters struct {

// –  The ID of the Data Catalog in which to create the connection. If none is supplied, the AWS account ID is used by default.
// +kubebuilder:validation:Optional
CatalogID *string `json:"catalogId,omitempty" tf:"catalog_id,omitempty"`

// value pairs used as parameters for this connection.
// +kubebuilder:validation:Optional
ConnectionPropertiesSecretRef *map[string]github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector%!(EXTRA string=v1) `json:"connectionPropertiesSecretRef,omitempty" tf:"-"`

// –  The type of the connection. Supported are: JDBC, MONGODB, KAFKA, and NETWORK. Defaults to JBDC.
// +kubebuilder:validation:Optional
ConnectionType *string `json:"connectionType,omitempty" tf:"connection_type,omitempty"`

// –  Description of the connection.
// +kubebuilder:validation:Optional
Description *string `json:"description,omitempty" tf:"description,omitempty"`

// –  A list of criteria that can be used in selecting this connection.
// +kubebuilder:validation:Optional
MatchCriteria []*string `json:"matchCriteria,omitempty" tf:"match_criteria,omitempty"`

// A map of physical connection requirements, such as VPC and SecurityGroup. Defined below.
// +kubebuilder:validation:Optional
PhysicalConnectionRequirements []PhysicalConnectionRequirementsParameters `json:"physicalConnectionRequirements,omitempty" tf:"physical_connection_requirements,omitempty"`

// Region is the region you'd like your resource to be created in.
// +upjet:crd:field:TFTag=-
// +kubebuilder:validation:Required
Region *string `json:"region" tf:"-"`

// Key-value map of resource tags. If configured with a provider default_tags configuration block present, tags with matching keys will overwrite those defined at the provider-level.
// +kubebuilder:validation:Optional
Tags map[string]*string `json:"tags,omitempty" tf:"tags,omitempty"`
}

type PhysicalConnectionRequirementsObservation struct {

}

type PhysicalConnectionRequirementsParameters struct {

// The availability zone of the connection. This field is redundant and implied by subnet_id, but is currently an api requirement.
// +crossplane:generate:reference:type=github.com/upbound/provider-aws/apis/ec2/v1beta1.Subnet
// +crossplane:generate:reference:extractor=github.com/upbound/upjet/pkg/resource.ExtractParamPath("availability_zone",false)
// +kubebuilder:validation:Optional
AvailabilityZone *string `json:"availabilityZone,omitempty" tf:"availability_zone,omitempty"`

// Reference to a Subnet in ec2 to populate availabilityZone.
// +kubebuilder:validation:Optional
AvailabilityZoneRef *v1common.Reference `json:"availabilityZoneRef,omitempty" tf:"-"`

// Selector for a Subnet in ec2 to populate availabilityZone.
// +kubebuilder:validation:Optional
AvailabilityZoneSelector *v1common.Selector `json:"availabilityZoneSelector,omitempty" tf:"-"`

// The security group ID list used by the connection.
// +kubebuilder:validation:Optional
SecurityGroupIDList []*string `json:"securityGroupIdList,omitempty" tf:"security_group_id_list,omitempty"`

// The subnet ID used by the connection.
// +crossplane:generate:reference:type=github.com/upbound/provider-aws/apis/ec2/v1beta1.Subnet
// +crossplane:generate:reference:extractor=github.com/upbound/upjet/pkg/resource.ExtractResourceID()
// +kubebuilder:validation:Optional
SubnetID *string `json:"subnetId,omitempty" tf:"subnet_id,omitempty"`

// Reference to a Subnet in ec2 to populate subnetId.
// +kubebuilder:validation:Optional
SubnetIDRef *v1common.Reference `json:"subnetIdRef,omitempty" tf:"-"`

// Selector for a Subnet in ec2 to populate subnetId.
// +kubebuilder:validation:Optional
SubnetIDSelector *v1common.Selector `json:"subnetIdSelector,omitempty" tf:"-"`
}

// ConnectionSpec defines the desired state of Connection
type ConnectionSpec struct {
    v1common.ResourceSpec `json:",inline"`
    ForProvider       ConnectionParameters `json:"forProvider"`
}

// ConnectionStatus defines the observed state of Connection.
type ConnectionStatus struct {
    v1common.ResourceStatus `json:",inline"`
    AtProvider          ConnectionObservation `json:"atProvider,omitempty"`
}

// +kubebuilder:object:root=true

// Connection is the Schema for the Connections API. Provides an Glue Connection resource.
// +kubebuilder:printcolumn:name="READY",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
// +kubebuilder:printcolumn:name="SYNCED",type="string",JSONPath=".status.conditions[?(@.type=='Synced')].status"
// +kubebuilder:printcolumn:name="EXTERNAL-NAME",type="string",JSONPath=".metadata.annotations.crossplane\\.io/external-name"
// +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp"
// +kubebuilder:subresource:status
// +kubebuilder:resource:scope=Cluster,categories={crossplane,managed,aws}
type Connection struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata,omitempty"`
    Spec              ConnectionSpec   `json:"spec"`
    Status            ConnectionStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// ConnectionList contains a list of Connections
type ConnectionList struct {
    metav1.TypeMeta `json:",inline"`
    metav1.ListMeta `json:"metadata,omitempty"`
    Items           []Connection `json:"items"`
}

// Repository type metadata.
var (
    Connection_Kind             = "Connection"
    Connection_GroupKind        = schema.GroupKind{Group: CRDGroup, Kind: Connection_Kind}.String()
    Connection_KindAPIVersion   = Connection_Kind + "." + CRDGroupVersion.String()
    Connection_GroupVersionKind = CRDGroupVersion.WithKind(Connection_Kind)
)

func init() {
    SchemeBuilder.Register(&Connection{}, &ConnectionList{})
}

How can we reproduce it?

jeanduplessis commented 1 year ago

@muvaf would appreciate if you can take a look at this issue next week.

muvaf commented 1 year ago

Looks like we're generating the following:

// value pairs used as parameters for this connection.
// +kubebuilder:validation:Optional
ConnectionPropertiesSecretRef *map[string]github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector%!(EXTRA string=v1) `json:"connectionPropertiesSecretRef,omitempty" tf:"-"`

for the TF schema given below:

            "connection_properties": {
                Type:         schema.TypeMap,
                Optional:     true,
                Sensitive:    true,
                ValidateFunc: mapKeyInSlice(glue.ConnectionPropertyKey_Values(), false),
                Elem:         &schema.Schema{Type: schema.TypeString},
            },

It's a bug in Upjet that it's confused seeing a map whose values are sensitive. Though I remember we added support for this case. I'll dig more and update this issue. I don't know of a workaround unfortunately, other than marking it as non-sensitive which defeats the purpose.

stevendborrelli commented 1 year ago

One related issue might be the parsing of the config/provider-metadata.yaml file.

This line in the metadata file:

connection_properties: – (Optional) A map of key-value pairs used as parameters for this connection.

results in this comment (note all the text before -value is truncated)

// value pairs used as parameters for this connection.

If I remove the dash in key-value:

connection_properties: – (Optional) A map of key value pairs used as parameters for this connection.

The full comment is generated:

// –  A map of key value pairs used as parameters for this connection.
muvaf commented 1 year ago

I opened https://github.com/upbound/upjet/pull/135 which fixes the problem but I think the pattern we have there is likely not in line with API conventions https://github.com/upbound/upjet/issues/134 . I'd like us consider changing it before publishing v1beta1 of aws_glue_connection and having to make a breaking change later. @stevendborrelli We can release AWS as soon as we settle on a decision there without having to wait for Monday release. Does that work for you?

stevendborrelli commented 1 year ago

@muvaf works for me!

turkenh commented 1 year ago

Fixed with https://github.com/upbound/upjet/pull/138