Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Upbound auth source and support for federated identity #206

Merged
merged 8 commits into from
Feb 6, 2023

Conversation

hasheddan
Copy link
Member

@hasheddan hasheddan commented Feb 2, 2023

Description of your changes

Adds an Upbound identity source to ProviderConfig to support federated
identity via OIDC.

Signed-off-by: hasheddan georgedanielmangum@gmail.com

TODO:
- Add example manifests

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested creating a bucket and was able to do so successfully. Also verified various invalid conditions:

  1. The attached service account does not have enough permissions.
    - lastTransitionTime: "2023-02-02T20:18:25Z"
      message: 'apply failed: googleapi: Error 403: proidc-dev-test@crossplane-playground.iam.gserviceaccount.com
        does not have storage.buckets.create access to the Google Cloud project. Permission
        ''storage.buckets.create'' denied on resource (or it may not exist)., forbidden: '
      reason: ApplyFailure
      status: "False"
      type: LastAsyncOperation
  1. The audience of the token is invalid.
    - lastTransitionTime: "2023-02-02T20:34:36Z"
      message: 'observe failed: cannot run refresh: refresh failed: Error when reading
        or editing Storage Bucket "really-cool-proidc-test": Get "https://storage.googleapis.com/storage/v1/b/really-cool-proidc-test?alt=json&prettyPrint=false":
        oauth2/google: unable to generate access token: Post "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/proidc-dev-test@crossplane-playground.iam.gserviceaccount.com:generateAccessToken":
        oauth2/google: status code 400: {"error":"invalid_grant","error_description":"The
        audience in ID Token [sts.amazonaws.com] does not match the expected audience
        sts.amazonaws.coms."}: '
      reason: ReconcileError
      status: "False"
      type: Synced
  1. The token does not satisfy constraints for performing the exchange.
    - lastTransitionTime: "2023-02-02T20:26:13Z"
      message: 'observe failed: cannot run refresh: refresh failed: Error when reading
        or editing Storage Bucket "really-cool-proidc-test": Get "https://storage.googleapis.com/storage/v1/b/really-cool-proidc-test?alt=json&prettyPrint=false":
        oauth2/google: unable to generate access token: Post "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/proidc-dev-test@crossplane-playground.iam.gserviceaccount.com:generateAccessToken":
        oauth2/google: status code 400: {"error":"unauthorized_client","error_description":"The
        given credential is rejected by the attribute condition."}: '
      reason: ReconcileError
      status: "False"
      type: Synced

Adds an Upbound identity source to ProviderConfig to support federated
identity via OIDC.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Generates updated fields on ProviderConfig for the Upbound auth source.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Constructs client configuration credentials for federated identity when
Upbound auth source is used.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Regenerates the list of generated resources.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
@@ -1 +1 @@
["google_active_directory_domain","google_apigee_envgroup","google_apigee_environment","google_apigee_instance","google_apigee_nat_address","google_apigee_organization","google_app_engine_application","google_app_engine_application_url_dispatch_rules","google_app_engine_service_network_settings","google_app_engine_standard_app_version","google_artifact_registry_repository","google_beyondcorp_app_connection","google_beyondcorp_app_connector","google_beyondcorp_app_gateway","google_bigquery_analytics_hub_data_exchange","google_bigquery_analytics_hub_listing","google_bigquery_connection","google_bigquery_data_transfer_config","google_bigquery_dataset","google_bigquery_dataset_access","google_bigquery_dataset_iam_binding","google_bigquery_dataset_iam_member","google_bigquery_dataset_iam_policy","google_bigquery_job","google_bigquery_reservation","google_bigquery_reservation_assignment","google_bigquery_routine","google_bigquery_table","google_bigquery_table_iam_binding","google_bigquery_table_iam_member","google_bigquery_table_iam_policy","google_bigtable_app_profile","google_bigtable_gc_policy","google_bigtable_instance","google_bigtable_instance_iam_binding","google_bigtable_instance_iam_member","google_bigtable_instance_iam_policy","google_bigtable_table","google_bigtable_table_iam_binding","google_bigtable_table_iam_member","google_bigtable_table_iam_policy","google_binary_authorization_attestor","google_binary_authorization_policy","google_certificate_manager_certificate_map","google_cloud_ids_endpoint","google_cloud_run_domain_mapping","google_cloud_run_service","google_cloud_run_service_iam_member","google_cloud_run_v2_job","google_cloud_run_v2_service","google_cloud_scheduler_job","google_cloud_tasks_queue","google_cloudbuild_trigger","google_cloudbuild_worker_pool","google_cloudfunctions_function","google_cloudfunctions_function_iam_member","google_cloudiot_device","google_cloudiot_registry","google_composer_environment","google_compute_address","google_compute_attached_disk","google_compute_autoscaler","google_compute_backend_bucket","google_compute_backend_bucket_signed_url_key","google_compute_backend_service","google_compute_disk","google_compute_disk_iam_member","google_compute_disk_resource_policy_attachment","google_compute_external_vpn_gateway","google_compute_firewall","google_compute_firewall_policy","google_compute_firewall_policy_association","google_compute_firewall_policy_rule","google_compute_forwarding_rule","google_compute_global_address","google_compute_global_forwarding_rule","google_compute_global_network_endpoint","google_compute_global_network_endpoint_group","google_compute_ha_vpn_gateway","google_compute_health_check","google_compute_http_health_check","google_compute_https_health_check","google_compute_image","google_compute_image_iam_member","google_compute_instance","google_compute_instance_from_template","google_compute_instance_group","google_compute_instance_group_manager","google_compute_instance_group_named_port","google_compute_instance_iam_member","google_compute_instance_template","google_compute_interconnect_attachment","google_compute_managed_ssl_certificate","google_compute_network","google_compute_network_endpoint","google_compute_network_endpoint_group","google_compute_network_peering","google_compute_network_peering_routes_config","google_compute_node_group","google_compute_node_template","google_compute_packet_mirroring","google_compute_per_instance_config","google_compute_project_default_network_tier","google_compute_project_metadata","google_compute_project_metadata_item","google_compute_region_autoscaler","google_compute_region_backend_service","google_compute_region_disk","google_compute_region_disk_iam_member","google_compute_region_disk_resource_policy_attachment","google_compute_region_health_check","google_compute_region_instance_group_manager","google_compute_region_network_endpoint_group","google_compute_region_per_instance_config","google_compute_region_ssl_certificate","google_compute_region_target_http_proxy","google_compute_region_target_https_proxy","google_compute_region_url_map","google_compute_reservation","google_compute_resource_policy","google_compute_route","google_compute_router","google_compute_router_interface","google_compute_router_nat","google_compute_security_policy","google_compute_service_attachment","google_compute_ssl_certificate","google_compute_subnetwork","google_compute_subnetwork_iam_member","google_compute_target_grpc_proxy","google_compute_target_http_proxy","google_compute_target_https_proxy","google_compute_target_instance","google_compute_target_pool","google_compute_target_ssl_proxy","google_compute_target_tcp_proxy","google_compute_url_map","google_compute_vpn_gateway","google_compute_vpn_tunnel","google_container_analysis_note","google_container_aws_cluster","google_container_aws_node_pool","google_container_azure_client","google_container_azure_cluster","google_container_azure_node_pool","google_container_cluster","google_container_node_pool","google_container_registry","google_data_catalog_entry","google_data_catalog_entry_group","google_data_catalog_tag","google_data_catalog_tag_template","google_data_fusion_instance","google_data_loss_prevention_deidentify_template","google_data_loss_prevention_inspect_template","google_data_loss_prevention_job_trigger","google_data_loss_prevention_stored_info_type","google_dataflow_job","google_dataplex_asset","google_dataplex_lake","google_dataplex_zone","google_dataproc_autoscaling_policy","google_dataproc_cluster","google_dataproc_job","google_dataproc_workflow_template","google_datastore_index","google_datastream_connection_profile","google_datastream_private_connection","google_dialogflow_cx_agent","google_dialogflow_cx_entity_type","google_dialogflow_cx_environment","google_dialogflow_cx_flow","google_dialogflow_cx_intent","google_dialogflow_cx_page","google_dialogflow_cx_version","google_dialogflow_cx_webhook","google_dns_managed_zone","google_dns_policy","google_dns_record_set","google_document_ai_processor","google_essential_contacts_contact","google_eventarc_trigger","google_filestore_backup","google_filestore_instance","google_filestore_snapshot","google_firebaserules_release","google_firebaserules_ruleset","google_folder","google_folder_iam_member","google_gke_hub_membership","google_healthcare_consent_store","google_healthcare_dataset","google_iap_web_backend_service_iam_member","google_iap_web_iam_member","google_iap_web_type_app_engine_iam_member","google_iap_web_type_compute_iam_member","google_identity_platform_default_supported_idp_config","google_identity_platform_inbound_saml_config","google_identity_platform_oauth_idp_config","google_identity_platform_project_default_config","google_identity_platform_tenant","google_identity_platform_tenant_default_supported_idp_config","google_identity_platform_tenant_inbound_saml_config","google_identity_platform_tenant_oauth_idp_config","google_kms_crypto_key","google_kms_crypto_key_iam_member","google_kms_crypto_key_version","google_kms_key_ring","google_kms_key_ring_iam_member","google_kms_key_ring_import_job","google_kms_secret_ciphertext","google_logging_log_view","google_logging_metric","google_logging_project_bucket_config","google_logging_project_exclusion","google_logging_project_sink","google_memcache_instance","google_ml_engine_model","google_monitoring_alert_policy","google_monitoring_custom_service","google_monitoring_dashboard","google_monitoring_group","google_monitoring_metric_descriptor","google_monitoring_notification_channel","google_monitoring_service","google_monitoring_slo","google_monitoring_uptime_check_config","google_network_connectivity_hub","google_network_connectivity_spoke","google_network_management_connectivity_test","google_notebooks_environment","google_notebooks_instance","google_notebooks_instance_iam_member","google_notebooks_runtime","google_notebooks_runtime_iam_member","google_organization_iam_audit_config","google_organization_iam_custom_role","google_organization_iam_member","google_os_config_os_policy_assignment","google_os_config_patch_deployment","google_os_login_ssh_public_key","google_privateca_ca_pool","google_privateca_ca_pool_iam_member","google_privateca_certificate","google_privateca_certificate_authority","google_privateca_certificate_template","google_privateca_certificate_template_iam_member","google_project","google_project_default_service_accounts","google_project_iam_audit_config","google_project_iam_member","google_project_service","google_project_usage_export_bucket","google_pubsub_lite_reservation","google_pubsub_lite_subscription","google_pubsub_lite_topic","google_pubsub_schema","google_pubsub_subscription","google_pubsub_subscription_iam_member","google_pubsub_topic","google_pubsub_topic_iam_member","google_redis_instance","google_secret_manager_secret","google_secret_manager_secret_iam_member","google_secret_manager_secret_version","google_service_account","google_service_account_iam_member","google_service_account_key","google_service_networking_connection","google_service_networking_peered_dns_domain","google_sourcerepo_repository","google_sourcerepo_repository_iam_member","google_spanner_database","google_spanner_database_iam_member","google_spanner_instance","google_spanner_instance_iam_member","google_sql_database","google_sql_database_instance","google_sql_source_representation_instance","google_sql_ssl_cert","google_sql_user","google_storage_bucket","google_storage_bucket_access_control","google_storage_bucket_acl","google_storage_bucket_iam_member","google_storage_bucket_object","google_storage_default_object_access_control","google_storage_default_object_acl","google_vertex_ai_dataset","google_vertex_ai_featurestore","google_vertex_ai_featurestore_entitytype","google_vertex_ai_tensorboard"]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff appears to be unrelated to changes here -- same check was failing on master.

Adds a note to TerraformSetupBuilder that we are slightly over our
cyclomatic complexity goal.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Adds an example manifest for a ProviderConfig using Upbound as an auth
source with federated identity.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
@hasheddan hasheddan requested a review from ulucinar February 2, 2023 20:51
Copy link
Contributor

@ezgidemirel ezgidemirel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @hasheddan, the change LGTM. But I leave the decision to @ulucinar and @sergenyalcin :)

// provider.
type Upbound struct {
// Federation is the configuration for federated identity.
Federation *Federation `json:"federation"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Federation *Federation `json:"federation"`
Federation *Federation `json:"federation,omitempty"`

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch!

TokenURL: "https://sts.googleapis.com/v1/token",
ServiceAccountImpersonationURL: fmt.Sprintf("https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/%s:generateAccessToken", serviceAccount),
CredentialSource: credentialFileSource{
File: upboundProviderIdentityTokenFile,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the credential file does not contain the name of the service account, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulucinar correct -- the mechanics are somewhat subtle: the file being referenced is the Upbound identity token, and we are using it to construct the GCP federated identity data (which is usually stored as a file, but we are just constructing it in-memory). So the credential source file here is just a JWT.

// ProviderID is the fully-qualified identifier for the identity provider on
// GCP. The format is
// `project/<project-id>/locations/global/workloadIdentityPools/<identity-pool>/providers/<identity-provider>`.
ProviderID string `json:"providerID"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we add some validation markers for these (at least to make sure they are non-empty)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulucinar I would love to -- what is y'all's current standard here? Just using validation such as min-length or have y'all started incorporating CEL experessions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe just a length check would be great for now if you also think it's worth. I don't know of any examples of regex checking or CEL-expressions yet. Unfortunately, we do not have a high standard here yet :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulucinar sounds reasonable -- thanks for the suggestion and context!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated!

Marks the federation config as omitemtpy.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Adds a minimum length requirement to federation values to ensure that
they are not empty strings.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
@hasheddan hasheddan requested a review from ulucinar February 6, 2023 14:29
Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @hasheddan, lgtm.

@hasheddan hasheddan merged commit 8f9f98e into crossplane-contrib:main Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants