From e6283ba3f2e175a9695feb8293acadd759786088 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 27 Jul 2026 08:21:39 -0700 Subject: [PATCH 1/9] chore(deps): bump coder/coder for codersdk.OAuth2ProviderSettings `codersdk.OAuth2ProviderSettings` and the `OAuth2ProviderSettings` / `PutOAuth2ProviderSettings` client methods are introduced by coder/coder#27316, which is required by the `coderd_oauth2_provider_settings` resource. TEMPORARY PIN: #27316 has not merged, so no released coder/coder version exposes these symbols. This pins the PR's head commit (569a0eb3411212ff080b836f514d1dfe4386ccc7, branch coder-eng-3056-dcr-flag) so the resource compiles and its tests run. Re-pin to a released version before merging. Transitively bumps the `go` directive to 1.26.5 and ~12 indirect dependencies. Refs #395 --- go.mod | 36 +++++++++++++++---------------- go.sum | 68 +++++++++++++++++++++++++++++++--------------------------- 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/go.mod b/go.mod index 90d9b62..df63b54 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ module github.com/coder/terraform-provider-coderd -go 1.26.4 +go 1.26.5 require ( cdr.dev/slog/v3 v3.1.0 - github.com/coder/coder/v2 v2.34.0-rc.0.0.20260713111656-06107768a8d6 + github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112 github.com/coder/retry v1.5.1 github.com/coder/serpent v0.15.0 github.com/coder/websocket v1.8.15 @@ -80,7 +80,7 @@ require ( github.com/containerd/errdefs/pkg v0.3.0 // indirect github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect - github.com/coreos/go-oidc/v3 v3.19.0 // indirect + github.com/coreos/go-oidc/v3 v3.20.0 // indirect github.com/cpuguy83/dockercfg v0.3.2 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect @@ -114,7 +114,7 @@ require ( github.com/hashicorp/hcl/v2 v2.24.0 // indirect github.com/hashicorp/logutils v1.0.0 // indirect github.com/hashicorp/terraform-exec v0.25.1 // indirect - github.com/hashicorp/terraform-json v0.27.3-0.20260213134036-298b8f6b673a // indirect + github.com/hashicorp/terraform-json v0.28.0 // indirect github.com/hashicorp/terraform-plugin-sdk/v2 v2.40.1 // indirect github.com/hashicorp/terraform-registry-address v0.4.0 // indirect github.com/hashicorp/terraform-svchost v0.2.1 // indirect @@ -167,10 +167,10 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/posener/complete v1.2.3 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect - github.com/prometheus/client_golang v1.23.2 // indirect + github.com/prometheus/client_golang v1.24.0 // indirect github.com/prometheus/client_model v0.6.2 // indirect - github.com/prometheus/common v0.69.0 // indirect - github.com/prometheus/procfs v0.20.1 // indirect + github.com/prometheus/common v0.70.0 // indirect + github.com/prometheus/procfs v0.21.1 // indirect github.com/puzpuzpuz/xsync/v3 v3.5.1 // indirect github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3 // indirect github.com/rivo/uniseg v0.4.7 // indirect @@ -217,22 +217,22 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.3 // indirect - golang.org/x/crypto v0.53.0 // indirect + golang.org/x/crypto v0.54.0 // indirect golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f // indirect - golang.org/x/mod v0.37.0 // indirect - golang.org/x/net v0.56.0 // indirect + golang.org/x/mod v0.38.0 // indirect + golang.org/x/net v0.57.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sync v0.21.0 // indirect - golang.org/x/sys v0.46.0 // indirect - golang.org/x/term v0.44.0 // indirect - golang.org/x/text v0.39.0 // indirect + golang.org/x/sync v0.22.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/term v0.45.0 // indirect + golang.org/x/text v0.40.0 // indirect golang.org/x/time v0.15.0 // indirect - golang.org/x/tools v0.47.0 // indirect + golang.org/x/tools v0.48.0 // indirect golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect google.golang.org/appengine v1.6.8 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20260610212136-7ab31c22f7ad // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260622175928-b703f567277d // indirect - google.golang.org/grpc v1.82.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800 // indirect + google.golang.org/grpc v1.82.1 // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/DataDog/dd-trace-go.v1 v1.74.0 // indirect gopkg.in/ini.v1 v1.67.3 // indirect diff --git a/go.sum b/go.sum index 8867ac3..98485db 100644 --- a/go.sum +++ b/go.sum @@ -136,8 +136,10 @@ github.com/clipperhouse/uax29/v2 v2.6.0 h1:z0cDbUV+aPASdFb2/ndFnS9ts/WNXgTNNGFoK github.com/clipperhouse/uax29/v2 v2.6.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260713111656-06107768a8d6 h1:zEKALHu+grAlyUJejp6XZoRGkvOQyWYIfY0WEWVXNy8= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260713111656-06107768a8d6/go.mod h1:imH39Jw6JBYWzFZtLx3juwWPhx+KxDqL3oPALrAh/U0= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260722122238-15be31e185b2 h1:WzI+01BOTkYee0l3h4uhD5K1/uBt5pVI8qGkyMCbpS4= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260722122238-15be31e185b2/go.mod h1:m+W7ywCko4nTGYYEo3vjp3RV6KuN80fiSui0nKCqbuI= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112 h1:gS3dUhwbomtRL/98vSZ3WXbagvCM4Nkl7ayG1173+TY= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112/go.mod h1:3ehot6+0lHQvdktT7D08k/vTAa0LaLeht8azzsjkRbw= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= @@ -156,8 +158,8 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpSBQv6A= github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= -github.com/coreos/go-oidc/v3 v3.19.0 h1:F/xyOi3x1UnG1U27YVnM1N6bHiL1K2upi6U/0qr8r+I= -github.com/coreos/go-oidc/v3 v3.19.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= +github.com/coreos/go-oidc/v3 v3.20.0 h1:EtE0WIBHk03N+DqGkY4+UONzzZHk7amKt6IyNd7OsZE= +github.com/coreos/go-oidc/v3 v3.20.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/cpuguy83/dockercfg v0.3.2 h1:DlJTyZGBDlXqUZ2Dk2Q3xHs/FtnooJJVaad2S9GKorA= github.com/cpuguy83/dockercfg v0.3.2/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= @@ -272,8 +274,8 @@ github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= github.com/hashicorp/terraform-exec v0.25.1 h1:PRutYRGM8pixV3B8812NYoBK5O+yuf3qcB/70KFKGiU= github.com/hashicorp/terraform-exec v0.25.1/go.mod h1:+izOYrs9sKMQK4OYvGDnrSSJHY/pm4e4eXFqSL2Q5mA= -github.com/hashicorp/terraform-json v0.27.3-0.20260213134036-298b8f6b673a h1:T7AMR21kjrbeEpN+KhGlyd31XXHsSZF5zg+ivfeYte4= -github.com/hashicorp/terraform-json v0.27.3-0.20260213134036-298b8f6b673a/go.mod h1:yjb5C2W07l8lmAzdyVgOLji0/D2IoHkR3rusBzUO4O0= +github.com/hashicorp/terraform-json v0.28.0 h1:dOkJT55rWfU6T1/VklHde51ym4LfNP+9xYR3ZizAJe4= +github.com/hashicorp/terraform-json v0.28.0/go.mod h1:PJIRf+Yzu5iLb52c/xYp1tUOL4jzMzfIAB5gvWWKIWE= github.com/hashicorp/terraform-plugin-docs v0.25.0 h1:qHs1V257NxVe8tv6HS4UQfNqjaPP5eUlLeDf7jYk85U= github.com/hashicorp/terraform-plugin-docs v0.25.0/go.mod h1:MQggCmY8zgP7R7E/cC0b0cmTvA9hSj3ZKyrrsDjRbLo= github.com/hashicorp/terraform-plugin-framework v1.19.0 h1:q0bwyhxAOR3vfdgbk9iplv3MlTv/dhBHTXjQOtQDoBA= @@ -439,14 +441,14 @@ github.com/posener/complete v1.2.3 h1:NP0eAhjcjImqslEwo/1hq7gpajME0fTLTezBKDqfXq github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSgv7Sy7s/s= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= -github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o= -github.com/prometheus/client_golang v1.23.2/go.mod h1:Tb1a6LWHB3/SPIzCoaDXI4I8UHKeFTEQ1YCr+0Gyqmg= +github.com/prometheus/client_golang v1.24.0 h1:5XStIklKuAtJSNpdD3s8XJj/Yv78IQmE1kbNk87JrAI= +github.com/prometheus/client_golang v1.24.0/go.mod h1:QcsNdotprC2nS4BTM2ucbcqxd2CeXTEa9jW7zHO9iDE= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= -github.com/prometheus/common v0.69.0 h1:OA85nJQS/T/MaYh/Q2CcgDKSGWqNIgrBDvDH85CuiNk= -github.com/prometheus/common v0.69.0/go.mod h1:ZzL3f6u94qUxh9p+tJTrF+FvBS1XXbbRAZCQkytAL0Y= -github.com/prometheus/procfs v0.20.1 h1:XwbrGOIplXW/AU3YhIhLODXMJYyC1isLFfYCsTEycfc= -github.com/prometheus/procfs v0.20.1/go.mod h1:o9EMBZGRyvDrSPH1RqdxhojkuXstoe4UlK79eF5TGGo= +github.com/prometheus/common v0.70.0 h1:bcpru3tWPVnxGnETLgOV5jbp/JRXgYEyv65CuBLAMMI= +github.com/prometheus/common v0.70.0/go.mod h1:S/SFasQmgGiYH6C81LKCtYa8QACgthGg5zxL2udV7SY= +github.com/prometheus/procfs v0.21.1 h1:GljZCt+zSTS+NZq88cyQ1LjZ+RCHp3uVuabBWA5+OJI= +github.com/prometheus/procfs v0.21.1/go.mod h1:aB55Cww9pdSJVHk0hUf0inxWyyjPogFIjmHKYgMKmtY= github.com/puzpuzpuz/xsync/v3 v3.5.1 h1:GJYJZwO6IdxN/IKbneznS6yPkVC+c3zyY/j19c++5Fg= github.com/puzpuzpuz/xsync/v3 v3.5.1/go.mod h1:VjzYrABPabuM4KyBh1Ftq6u8nhwY5tBPKP9jpmh0nnA= github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3 h1:4+LEVOB87y175cLJC/mbsgKmoDOjrBldtXvioEy96WY= @@ -640,15 +642,15 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.3.0/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw= -golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= -golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f h1:W3F4c+6OLc6H2lb//N1q4WpJkhzJCK5J6kUi1NTVXfM= golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f/go.mod h1:J1xhfL/vlindoeF/aINzNzt2Bket5bjo9sdOYzOsU80= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= -golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= +golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= +golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= @@ -659,8 +661,8 @@ golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI= -golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= -golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -668,8 +670,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= -golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -694,8 +696,8 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= -golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -703,8 +705,8 @@ golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU= -golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= -golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= @@ -713,8 +715,8 @@ golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= -golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= -golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -722,8 +724,8 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= -golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= +golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= +golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -737,12 +739,14 @@ google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAs google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= google.golang.org/genproto v0.0.0-20260610212136-7ab31c22f7ad h1:cYL1DPJAQr4JMvhfGao0PDXoaf03ifMljAuDyrbMBd0= google.golang.org/genproto v0.0.0-20260610212136-7ab31c22f7ad/go.mod h1:cVHIikDNAdx8ISZeW+2rYkEMf3xn0GSaBYmVnWXQBUo= -google.golang.org/genproto/googleapis/api v0.0.0-20260610212136-7ab31c22f7ad h1:3iLyITS/sySRwbUKoC7ogfj2Yr1Cjs0pfaRKj5U5HEw= -google.golang.org/genproto/googleapis/api v0.0.0-20260610212136-7ab31c22f7ad/go.mod h1:KdNqO+rCIWgFumrNBSEDlDNrkrQnpkax7Tv1WxNY8V4= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260622175928-b703f567277d h1:mpAgMyM9vQHxycBlDq50y1VHpfSfVwzXvrQKtYbXuUY= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260622175928-b703f567277d/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 h1:jQ9p21COKWjP3VwuFrNRiiOTMh3mPpN45R7SLrH/HUU= +google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7/go.mod h1:KqHwBx2upmfa1XSi1WuRvC+2VGCLtooKkfmyvRbUmqA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800 h1:qEHAMpSaUhtD0p3NbEEI83HwNGFxEwaSJ1G9PLnCBZE= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= google.golang.org/grpc v1.82.0 h1:vguDnZUPjE26w09A63VoxZPnvPjB5Riyc0mkXPFmAIU= google.golang.org/grpc v1.82.0/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= +google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= +google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= From e7f2ca0fbb2d023613d2852415b4d441e73d9408 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 27 Jul 2026 08:21:55 -0700 Subject: [PATCH 2/9] feat(coderd_oauth2_provider_settings): manage the OAuth2 DCR toggle coder/coder#27316 adds a deployment-level toggle for OAuth2 Dynamic Client Registration (RFC 7591), reachable via `coder oauth2-provider dcr enable|disable` or a raw `PUT /api/v2/oauth2-provider/settings`. Neither is declarative, so a deployment managed by this provider had no way to express "DCR is enabled here" as Terraform state, short of a `local-exec` provisioner that participates in neither plan/diff, drift detection, nor destroy. Add a `coderd_oauth2_provider_settings` resource and a data source of the same name, modelled on `coderd_organization_sync_settings`: a singleton wrapping a GET-for-read / PUT-for-write API with no delete endpoint. Resource: - `dynamic_client_registration_enabled` is Required. Opting out means not declaring the resource; a plain Optional attribute would refresh to a concrete bool against a null plan and produce a perpetual diff. - Create and Update share one idempotent PUT; the API has no separate create. - Delete resets to the documented default (false), since no DELETE verb exists. A failed reset keeps the resource in state so destroy can retry. - ImportState allows adopting an already-configured deployment without overwriting it. The import ID is an unused placeholder: the resource is a deployment-wide singleton and Read() takes no parameters. - ModifyPlan warns at plan time when a first apply would disable DCR on a deployment where it is currently enabled. Deliberately asymmetric: a live `false` is indistinguishable from never-configured, so warning in that direction would fire on every greenfield apply. - A 404 is reported as an unsupported Coder version rather than treated as a deleted resource. This singleton always exists on a supported deployment, so 404 can only mean the endpoint is missing; removing state would hide a version problem behind a phantom diff. Data source: read-only, GET only, never PUT. The setting is a deployment singleton, so only one configuration can own the resource; any other that needs the value reads it here. It also works with tokens holding read but not write access on the deployment config, a broader set of roles than the resource requires. `DynamicClientRegistrationEnabled` is a `*bool`, so that a PUT can omit it to leave the value unchanged. Both write paths therefore send an explicit non-nil pointer: this resource owns the value outright, and omitting it would make Create/Update no-ops and silently turn Delete's reset-to-default into a no-op. Reads go through `dcrEnabledOrDefault`, which falls back to the deployment default if the field is ever nil -- a GET is documented to always return non-nil, so that branch is defensive against a contract violation. Tests run against a recording httptest fake rather than `integration.StartCoder`, which pulls a published coder image that does not yet contain #27316. The fake also allows asserting requests that must *not* happen -- import never PUTs, the data source never PUTs, an undeclared resource issues no calls at all -- and injecting 403/404/5xx responses that a real deployment will not produce on demand. It records whether the DCR field was present on each PUT, not just its value, so a regression that stopped sending it would fail rather than pass silently. Closes #395 --- docs/data-sources/oauth2_provider_settings.md | 67 ++ docs/resources/oauth2_provider_settings.md | 120 +++ .../data-source.tf | 21 + .../coderd_oauth2_provider_settings/import.sh | 17 + .../resource.tf | 26 + .../oauth2_provider_settings_data_source.go | 103 +++ ...uth2_provider_settings_data_source_test.go | 162 ++++ .../oauth2_provider_settings_fake_test.go | 230 ++++++ .../oauth2_provider_settings_resource.go | 351 ++++++++ .../oauth2_provider_settings_resource_test.go | 771 ++++++++++++++++++ internal/provider/provider.go | 2 + 11 files changed, 1870 insertions(+) create mode 100644 docs/data-sources/oauth2_provider_settings.md create mode 100644 docs/resources/oauth2_provider_settings.md create mode 100644 examples/data-sources/coderd_oauth2_provider_settings/data-source.tf create mode 100644 examples/resources/coderd_oauth2_provider_settings/import.sh create mode 100644 examples/resources/coderd_oauth2_provider_settings/resource.tf create mode 100644 internal/provider/oauth2_provider_settings_data_source.go create mode 100644 internal/provider/oauth2_provider_settings_data_source_test.go create mode 100644 internal/provider/oauth2_provider_settings_fake_test.go create mode 100644 internal/provider/oauth2_provider_settings_resource.go create mode 100644 internal/provider/oauth2_provider_settings_resource_test.go diff --git a/docs/data-sources/oauth2_provider_settings.md b/docs/data-sources/oauth2_provider_settings.md new file mode 100644 index 0000000..a209838 --- /dev/null +++ b/docs/data-sources/oauth2_provider_settings.md @@ -0,0 +1,67 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "coderd_oauth2_provider_settings Data Source - terraform-provider-coderd" +subcategory: "" +description: |- + The deployment-wide OAuth2 provider settings. + Use this data source to read the current settings without taking ownership of + them. Because the settings are a deployment-wide singleton, only one Terraform + configuration can own the coderd_oauth2_provider_settings resource; any other + configuration that needs the value should read it here instead. + -> This data source only issues a GET, so it works with any token holding + read access on the deployment configuration, which is a broader set of roles + than the resource's write path requires. An Auditor token, for example, can + read this data source but cannot own the equivalent resource. + ~> Warning + This data source is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. +--- + +# coderd_oauth2_provider_settings (Data Source) + +The deployment-wide OAuth2 provider settings. + +Use this data source to read the current settings without taking ownership of +them. Because the settings are a deployment-wide singleton, only one Terraform +configuration can own the `coderd_oauth2_provider_settings` *resource*; any other +configuration that needs the value should read it here instead. + +-> This data source only issues a `GET`, so it works with any token holding +read access on the deployment configuration, which is a broader set of roles +than the resource's write path requires. An Auditor token, for example, can +read this data source but cannot own the equivalent resource. + +~> **Warning** +This data source is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. + +## Example Usage + +```terraform +// Read the deployment's OAuth2 provider settings without taking ownership of +// them. Use this when another configuration (or nobody at all) owns the +// `coderd_oauth2_provider_settings` resource, since the setting is a +// deployment-wide singleton and only one configuration can manage it. +// +// This only issues a GET, so it also works with tokens that can read but not +// write the deployment configuration, such as an Auditor token. +data "coderd_oauth2_provider_settings" "current" {} + +// Because Dynamic Client Registration is disabled, provision a static OAuth2 +// client instead, for MCP clients that cannot register themselves. +resource "coderd_group" "mcp_static_clients" { + count = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled ? 0 : 1 + + name = "mcp-static-clients" + organization_id = data.coderd_organization.default.id +} + +output "dynamic_client_registration_enabled" { + value = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled +} +``` + + +## Schema + +### Read-Only + +- `dynamic_client_registration_enabled` (Boolean) Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) is enabled for the deployment. A deployment that has never configured this setting reads back as `false`. diff --git a/docs/resources/oauth2_provider_settings.md b/docs/resources/oauth2_provider_settings.md new file mode 100644 index 0000000..46a2908 --- /dev/null +++ b/docs/resources/oauth2_provider_settings.md @@ -0,0 +1,120 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "coderd_oauth2_provider_settings Resource - terraform-provider-coderd" +subcategory: "" +description: |- + Deployment-wide OAuth2 provider settings. + This setting is a deployment-wide singleton, so this resource can only be + declared once. Declaring it more than once is not an error: whichever block + applies last silently wins. + ~> Warning + When adopting a deployment where this setting has already been configured out + of band (via coder oauth2-provider dcr enable or the deployment settings UI), run + terraform import before your first terraform apply. There is no prior state + for Terraform to diff against on a first apply, so the live value is overwritten + with your configured one without appearing as a change in the plan. A plan-time + warning is raised if this would disable Dynamic Client Registration where it is + currently enabled, but a warning does not block the apply. + ~> Warning + terraform destroy resets dynamic_client_registration_enabled to false, the + deployment default. The API has no delete operation for this setting, so the + value cannot be returned to a "never configured" state. + -> Managing this setting is entirely optional: omit the resource to leave + Dynamic Client Registration alone. To read the current value without taking + ownership of it, use the coderd_oauth2_provider_settings data source instead. + ~> Warning + This resource is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. +--- + +# coderd_oauth2_provider_settings (Resource) + +Deployment-wide OAuth2 provider settings. + +This setting is a deployment-wide singleton, so this resource can only be +declared once. Declaring it more than once is not an error: whichever block +applies last silently wins. + +~> **Warning** +When adopting a deployment where this setting has already been configured out +of band (via `coder oauth2-provider dcr enable` or the deployment settings UI), run +`terraform import` **before** your first `terraform apply`. There is no prior state +for Terraform to diff against on a first apply, so the live value is overwritten +with your configured one without appearing as a change in the plan. A plan-time +warning is raised if this would disable Dynamic Client Registration where it is +currently enabled, but a warning does not block the apply. + +~> **Warning** +`terraform destroy` resets `dynamic_client_registration_enabled` to `false`, the +deployment default. The API has no delete operation for this setting, so the +value cannot be returned to a "never configured" state. + +-> Managing this setting is entirely optional: omit the resource to leave +Dynamic Client Registration alone. To read the current value without taking +ownership of it, use the `coderd_oauth2_provider_settings` data source instead. + +~> **Warning** +This resource is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. + +## Example Usage + +```terraform +// Important note: this setting is a deployment-wide singleton, so you can only +// have one resource of this type! Declaring it twice is not an error, the +// blocks will silently overwrite each other on every apply. +// +// If the deployment already has this setting configured (via the CLI or the +// deployment settings UI), run `terraform import` before your first apply, or +// this resource will overwrite the live value without showing a diff. +resource "coderd_oauth2_provider_settings" "dcr" { + dynamic_client_registration_enabled = true + + // Only needed when the same configuration also upgrades Coder itself. + // `/api/v2/oauth2-provider/settings` does not exist until the new version is + // actually serving, and `depends_on = [helm_release.coder]` alone only orders + // "the Helm apply returned", which for a rolling update can still leave + // old-version pods behind the load balancer. Gate on the API responding + // instead, or apply the upgrade and this resource in two separate runs. + depends_on = [terraform_data.coder_ready] +} + +resource "terraform_data" "coder_ready" { + depends_on = [helm_release.coder] + + provisioner "local-exec" { + command = "until curl -sfo /dev/null $CODER_URL/api/v2/buildinfo; do sleep 5; done" + } +} +``` + + +## Schema + +### Required + +- `dynamic_client_registration_enabled` (Boolean) Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) is enabled for the deployment. When disabled, `POST /oauth2/register` is rejected and the `registration_endpoint` is omitted from the authorization server metadata document. + +## Import + +Import is supported using the following syntax: + +The [`terraform import` command](https://developer.hashicorp.com/terraform/cli/commands/import) can be used, for example: + +```shell +# This setting is a deployment-wide singleton with no identifying attribute, so +# the import ID is an unused placeholder. Any string works; a fixed one keeps +# the command copy-pasteable. +# +# Import before your first apply when adopting a deployment that already has +# this setting configured: state is then populated from the live value, and the +# following `terraform plan` shows an honest diff instead of silently +# overwriting it. +$ terraform import coderd_oauth2_provider_settings.dcr oauth2_provider_settings +``` +Alternatively, in Terraform v1.5.0 and later, an [`import` block](https://developer.hashicorp.com/terraform/language/import) can be used: + +```terraform +import { + to = coderd_oauth2_provider_settings.dcr + id = "oauth2_provider_settings" +} +``` diff --git a/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf b/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf new file mode 100644 index 0000000..7ab8bff --- /dev/null +++ b/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf @@ -0,0 +1,21 @@ +// Read the deployment's OAuth2 provider settings without taking ownership of +// them. Use this when another configuration (or nobody at all) owns the +// `coderd_oauth2_provider_settings` resource, since the setting is a +// deployment-wide singleton and only one configuration can manage it. +// +// This only issues a GET, so it also works with tokens that can read but not +// write the deployment configuration, such as an Auditor token. +data "coderd_oauth2_provider_settings" "current" {} + +// Because Dynamic Client Registration is disabled, provision a static OAuth2 +// client instead, for MCP clients that cannot register themselves. +resource "coderd_group" "mcp_static_clients" { + count = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled ? 0 : 1 + + name = "mcp-static-clients" + organization_id = data.coderd_organization.default.id +} + +output "dynamic_client_registration_enabled" { + value = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled +} diff --git a/examples/resources/coderd_oauth2_provider_settings/import.sh b/examples/resources/coderd_oauth2_provider_settings/import.sh new file mode 100644 index 0000000..23f00da --- /dev/null +++ b/examples/resources/coderd_oauth2_provider_settings/import.sh @@ -0,0 +1,17 @@ +# This setting is a deployment-wide singleton with no identifying attribute, so +# the import ID is an unused placeholder. Any string works; a fixed one keeps +# the command copy-pasteable. +# +# Import before your first apply when adopting a deployment that already has +# this setting configured: state is then populated from the live value, and the +# following `terraform plan` shows an honest diff instead of silently +# overwriting it. +$ terraform import coderd_oauth2_provider_settings.dcr oauth2_provider_settings +``` +Alternatively, in Terraform v1.5.0 and later, an [`import` block](https://developer.hashicorp.com/terraform/language/import) can be used: + +```terraform +import { + to = coderd_oauth2_provider_settings.dcr + id = "oauth2_provider_settings" +} diff --git a/examples/resources/coderd_oauth2_provider_settings/resource.tf b/examples/resources/coderd_oauth2_provider_settings/resource.tf new file mode 100644 index 0000000..1a297b8 --- /dev/null +++ b/examples/resources/coderd_oauth2_provider_settings/resource.tf @@ -0,0 +1,26 @@ +// Important note: this setting is a deployment-wide singleton, so you can only +// have one resource of this type! Declaring it twice is not an error, the +// blocks will silently overwrite each other on every apply. +// +// If the deployment already has this setting configured (via the CLI or the +// deployment settings UI), run `terraform import` before your first apply, or +// this resource will overwrite the live value without showing a diff. +resource "coderd_oauth2_provider_settings" "dcr" { + dynamic_client_registration_enabled = true + + // Only needed when the same configuration also upgrades Coder itself. + // `/api/v2/oauth2-provider/settings` does not exist until the new version is + // actually serving, and `depends_on = [helm_release.coder]` alone only orders + // "the Helm apply returned", which for a rolling update can still leave + // old-version pods behind the load balancer. Gate on the API responding + // instead, or apply the upgrade and this resource in two separate runs. + depends_on = [terraform_data.coder_ready] +} + +resource "terraform_data" "coder_ready" { + depends_on = [helm_release.coder] + + provisioner "local-exec" { + command = "until curl -sfo /dev/null $CODER_URL/api/v2/buildinfo; do sleep 5; done" + } +} diff --git a/internal/provider/oauth2_provider_settings_data_source.go b/internal/provider/oauth2_provider_settings_data_source.go new file mode 100644 index 0000000..6ce31aa --- /dev/null +++ b/internal/provider/oauth2_provider_settings_data_source.go @@ -0,0 +1,103 @@ +package provider + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// Ensure provider defined types fully satisfy framework interfaces. +var _ datasource.DataSource = &OAuth2ProviderSettingsDataSource{} + +func NewOAuth2ProviderSettingsDataSource() datasource.DataSource { + return &OAuth2ProviderSettingsDataSource{} +} + +// OAuth2ProviderSettingsDataSource defines the data source implementation. +type OAuth2ProviderSettingsDataSource struct { + data *CoderdProviderData +} + +// OAuth2ProviderSettingsDataSourceModel describes the data source data model. +type OAuth2ProviderSettingsDataSourceModel struct { + DynamicClientRegistrationEnabled types.Bool `tfsdk:"dynamic_client_registration_enabled"` +} + +func (d *OAuth2ProviderSettingsDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_oauth2_provider_settings" +} + +func (d *OAuth2ProviderSettingsDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { + resp.Schema = schema.Schema{ + MarkdownDescription: `The deployment-wide OAuth2 provider settings. + +Use this data source to read the current settings without taking ownership of +them. Because the settings are a deployment-wide singleton, only one Terraform +configuration can own the ` + "`coderd_oauth2_provider_settings`" + ` *resource*; any other +configuration that needs the value should read it here instead. + +-> This data source only issues a ` + "`GET`" + `, so it works with any token holding +read access on the deployment configuration, which is a broader set of roles +than the resource's write path requires. An Auditor token, for example, can +read this data source but cannot own the equivalent resource. + +~> **Warning** +This data source is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. +`, + + Attributes: map[string]schema.Attribute{ + "dynamic_client_registration_enabled": schema.BoolAttribute{ + Computed: true, + MarkdownDescription: "Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) " + + "is enabled for the deployment. A deployment that has never configured this setting reads back as `false`.", + }, + }, + } +} + +func (d *OAuth2ProviderSettingsDataSource) Configure(ctx context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + // Prevent panic if the provider has not been configured. + if req.ProviderData == nil { + return + } + + data, ok := req.ProviderData.(*CoderdProviderData) + + if !ok { + resp.Diagnostics.AddError( + "Unexpected Data Source Configure Type", + fmt.Sprintf("Expected *CoderdProviderData, got: %T. Please report this issue to the provider developers.", req.ProviderData), + ) + + return + } + + d.data = data +} + +func (d *OAuth2ProviderSettingsDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { + var data OAuth2ProviderSettingsDataSourceModel + + // Read Terraform configuration data into the model + resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + // GET only. This data source must never call PutOAuth2ProviderSettings: + // that invariant is what makes it safe to declare alongside the resource + // without risking a write conflict. + settings, err := d.data.Client.OAuth2ProviderSettings(ctx) + if err != nil { + resp.Diagnostics.Append(oauth2ProviderSettingsDiag("read", err)...) + return + } + + data.DynamicClientRegistrationEnabled = types.BoolValue(dcrEnabledOrDefault(settings)) + + // Save data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} diff --git a/internal/provider/oauth2_provider_settings_data_source_test.go b/internal/provider/oauth2_provider_settings_data_source_test.go new file mode 100644 index 0000000..c1bd5bd --- /dev/null +++ b/internal/provider/oauth2_provider_settings_data_source_test.go @@ -0,0 +1,162 @@ +package provider + +import ( + "net/http" + "os" + "regexp" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" + "github.com/stretchr/testify/assert" +) + +const oauth2SettingsDataSourceAddr = "data.coderd_oauth2_provider_settings.test" + +func oauth2SettingsDataSourceConfig(url string) string { + return oauth2SettingsProviderBlock(url) + ` +data "coderd_oauth2_provider_settings" "test" {} +` +} + +// TestAccOAuth2ProviderSettingsDataSource covers the proposal's 5.7 group: the +// read-without-owning path. +func TestAccOAuth2ProviderSettingsDataSource(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + + // TC16 — Data source only, no resource declared. The invariant that makes + // it safe to declare alongside a resource owned elsewhere is that it never + // writes, asserted here against the test double rather than claimed in + // docs. + t.Run("TC16_ReadOnlyNeverPuts", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetDCREnabled(true) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), + }, + }, + { + // Repeated applies must stay read-only too. + Config: oauth2SettingsDataSourceConfig(f.URL), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), + }, + }, + }, + }) + + assert.Zero(t, f.SettingsRequestCount(http.MethodPut), "the data source must never issue a PUT") + assert.NotZero(t, f.SettingsRequestCount(http.MethodGet)) + assert.True(t, f.DCREnabled(), "the live value must be untouched") + }) + + // TC17 — Data source with a token that lacks read on the deployment + // config: the 403 surfaces as an error, not a silently zero-valued + // attribute. + t.Run("TC17_Forbidden", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetGetStatus(http.StatusForbidden) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ExpectError: regexp.MustCompile(`(?s)unable to read OAuth2 provider settings.*Forbidden`), + }, + }, + }) + }) + + // TC18 — Data source against an old Coderd: the same 404-derived error as + // TC11, since both route through Client.OAuth2ProviderSettings. + t.Run("TC18_OldCoderd", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetGetStatus(http.StatusNotFound) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ExpectError: regexp.MustCompile(`Unsupported Coder Version`), + }, + }, + }) + }) + + // TC19 — Fresh deployment, never configured, observed through the data + // source only. The value reaches state even though no configuration + // anywhere supplied one, because the data source's only attribute is + // Computed. + t.Run("TC19_FreshDeploymentDefaultsFalse", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(false)), + }, + Check: func(*terraform.State) error { + assert.Zero(t, f.SettingsRequestCount(http.MethodPut)) + return nil + }, + }, + }, + }) + }) + + // TC24 — Auditor-role token: the data source resolves fine even though the + // same token could not own the equivalent resource (TC23). The data source + // only ever calls the read-gated GET, so the write-gated 403 is never + // reached. + t.Run("TC24_ReadableByTokenThatCannotWrite", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetDCREnabled(true) + f.SetPutStatus(http.StatusForbidden) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), + }, + }, + }, + }) + + assert.Zero(t, f.SettingsRequestCount(http.MethodPut), + "the forbidden write path must never be exercised by the data source") + }) +} diff --git a/internal/provider/oauth2_provider_settings_fake_test.go b/internal/provider/oauth2_provider_settings_fake_test.go new file mode 100644 index 0000000..89ab1ac --- /dev/null +++ b/internal/provider/oauth2_provider_settings_fake_test.go @@ -0,0 +1,230 @@ +package provider + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/coder/coder/v2/codersdk" +) + +// oauth2SettingsPath is the endpoint added by coder/coder#27316. +const oauth2SettingsPath = "/api/v2/oauth2-provider/settings" + +// fakeCoderd is a minimal stand-in for a Coder deployment, serving just the +// endpoints the provider touches: the two `Configure()` calls plus the OAuth2 +// provider settings singleton. +// +// A fake rather than `integration.StartCoder` because most of the OAuth2 +// settings test matrix is about what the *provider* does with a given API +// response: that a 404 becomes a version hint, that a 403 becomes a clean +// diagnostic, that import issues no PUT at all. Those are assertions about +// requests made and not made, which a real deployment cannot report. +type fakeCoderd struct { + *httptest.Server + + mu sync.Mutex + dcrEnabled bool + requests []fakeRequest + + // getStatus and putStatus, when non-zero, make the settings endpoint fail + // with that status instead of behaving normally. Used to model an old + // deployment (404), an unauthorized token (403), and a transient outage + // (500). The provider cannot distinguish "the token lacks the role" from + // "the API said 403", which is exactly the contract under test: the RBAC + // decision itself belongs to coderd, not here. + getStatus int + putStatus int + + // beforeGet runs before each settings GET is answered, and beforeSettingsPut + // before each PUT. Both are used to simulate another actor mutating the + // deployment mid-apply. + beforeGet func(f *fakeCoderd) + beforePut func(f *fakeCoderd) +} + +type fakeRequest struct { + Method string + Path string + // DCREnabled is the body value on a settings PUT, valid only when DCRSent + // is true. + DCREnabled bool + // DCRSent records whether the PUT body carried the field at all. The API + // reads an omitted field as "leave the current value alone", so "sent + // false" and "omitted" are different requests with different effects. + DCRSent bool +} + +func newFakeCoderd(t *testing.T) *fakeCoderd { + t.Helper() + + f := &fakeCoderd{} + f.Server = httptest.NewServer(http.HandlerFunc(f.handle)) + t.Cleanup(f.Close) + return f +} + +func (f *fakeCoderd) handle(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + f.requests = append(f.requests, fakeRequest{Method: r.Method, Path: r.URL.Path}) + f.mu.Unlock() + + switch { + case r.URL.Path == "/api/v2/users/me": + writeJSON(w, http.StatusOK, map[string]any{ + "id": "00000000-0000-0000-0000-000000000001", + "username": "admin", + "organization_ids": []string{"00000000-0000-0000-0000-000000000002"}, + }) + case r.URL.Path == "/api/v2/entitlements": + writeJSON(w, http.StatusOK, codersdk.Entitlements{ + Features: map[codersdk.FeatureName]codersdk.Feature{}, + }) + case r.URL.Path == oauth2SettingsPath && r.Method == http.MethodGet: + f.handleSettingsGet(w) + case r.URL.Path == oauth2SettingsPath && r.Method == http.MethodPut: + f.handleSettingsPut(w, r) + default: + writeJSON(w, http.StatusNotFound, codersdk.Response{Message: "Not Found."}) + } +} + +func (f *fakeCoderd) handleSettingsGet(w http.ResponseWriter) { + f.mu.Lock() + hook := f.beforeGet + f.mu.Unlock() + if hook != nil { + hook(f) + } + + f.mu.Lock() + status, enabled := f.getStatus, f.dcrEnabled + f.mu.Unlock() + + if status != 0 { + writeJSON(w, status, codersdk.Response{Message: statusMessage(status)}) + return + } + // A GET always answers with a non-nil pointer, matching the documented + // contract on codersdk.OAuth2ProviderSettings. + writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{DynamicClientRegistrationEnabled: &enabled}) +} + +func (f *fakeCoderd) handleSettingsPut(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + hook := f.beforePut + status := f.putStatus + f.mu.Unlock() + if hook != nil { + hook(f) + } + + var body codersdk.OAuth2ProviderSettings + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeJSON(w, http.StatusBadRequest, codersdk.Response{Message: "Malformed body.", Detail: err.Error()}) + return + } + + f.mu.Lock() + // Record what was actually sent, so assertions can check both the value + // and whether the field was present at all. A nil field means the caller + // omitted it, which the real API treats as "leave the current value + // alone" -- mirrored here so a regression that stops sending the value + // (making Delete's reset a silent no-op) fails a test instead of passing. + f.requests[len(f.requests)-1].DCRSent = body.DynamicClientRegistrationEnabled != nil + if body.DynamicClientRegistrationEnabled != nil { + f.requests[len(f.requests)-1].DCREnabled = *body.DynamicClientRegistrationEnabled + if status == 0 { + f.dcrEnabled = *body.DynamicClientRegistrationEnabled + } + } + resolved := f.dcrEnabled + f.mu.Unlock() + + if status != 0 { + writeJSON(w, status, codersdk.Response{Message: statusMessage(status)}) + return + } + writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{DynamicClientRegistrationEnabled: &resolved}) +} + +// statusMessage mirrors the shape of the message coderd itself returns, so the +// provider's error text is assertable. +func statusMessage(status int) string { + switch status { + case http.StatusNotFound: + return "Not Found." + case http.StatusForbidden: + return "Forbidden." + default: + return fmt.Sprintf("Internal error (%d).", status) + } +} + +func writeJSON(w http.ResponseWriter, status int, body any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(body) +} + +// --- assertion helpers --- + +// SetDCREnabled sets the live value, standing in for an out-of-band change via +// the CLI, the deployment settings UI, or another Terraform run. +func (f *fakeCoderd) SetDCREnabled(v bool) { + f.mu.Lock() + defer f.mu.Unlock() + f.dcrEnabled = v +} + +func (f *fakeCoderd) DCREnabled() bool { + f.mu.Lock() + defer f.mu.Unlock() + return f.dcrEnabled +} + +func (f *fakeCoderd) SetGetStatus(status int) { + f.mu.Lock() + defer f.mu.Unlock() + f.getStatus = status +} + +func (f *fakeCoderd) SetPutStatus(status int) { + f.mu.Lock() + defer f.mu.Unlock() + f.putStatus = status +} + +func (f *fakeCoderd) SetBeforeGet(hook func(f *fakeCoderd)) { + f.mu.Lock() + defer f.mu.Unlock() + f.beforeGet = hook +} + +// SettingsRequests returns every request made to the settings endpoint, in +// order. +func (f *fakeCoderd) SettingsRequests() []fakeRequest { + f.mu.Lock() + defer f.mu.Unlock() + var out []fakeRequest + for _, req := range f.requests { + if req.Path == oauth2SettingsPath { + out = append(out, req) + } + } + return out +} + +// SettingsRequestCount counts settings requests with the given method. +func (f *fakeCoderd) SettingsRequestCount(method string) int { + n := 0 + for _, req := range f.SettingsRequests() { + if req.Method == method { + n++ + } + } + return n +} diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go new file mode 100644 index 0000000..db129e1 --- /dev/null +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -0,0 +1,351 @@ +package provider + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/coder/coder/v2/codersdk" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" +) + +// Ensure provider defined types fully satisfy framework interfaces. +var _ resource.Resource = &OAuth2ProviderSettingsResource{} +var _ resource.ResourceWithImportState = &OAuth2ProviderSettingsResource{} +var _ resource.ResourceWithModifyPlan = &OAuth2ProviderSettingsResource{} + +// oauth2ProviderSettingsMinVersion is the first Coder release exposing +// `/api/v2/oauth2-provider/settings`. It is named in the error surfaced when +// that endpoint 404s, so an admin pointed at an older deployment gets an +// actionable message instead of a bare "not found". +const oauth2ProviderSettingsMinVersion = "2.35.0" + +// oauth2ProviderSettingsDefaultDCR is the deployment default for +// `dynamic_client_registration_enabled`. A never-configured deployment reads +// back as `false`, and `Delete` restores this value because the API has no +// DELETE verb for the setting. +const oauth2ProviderSettingsDefaultDCR = false + +// pathDynamicClientRegistrationEnabled anchors plan-time diagnostics to the +// attribute they are about. +var pathDynamicClientRegistrationEnabled = path.Root("dynamic_client_registration_enabled") + +type OAuth2ProviderSettingsResource struct { + *CoderdProviderData +} + +// OAuth2ProviderSettingsResourceModel describes the resource data model. +type OAuth2ProviderSettingsResourceModel struct { + DynamicClientRegistrationEnabled types.Bool `tfsdk:"dynamic_client_registration_enabled"` +} + +func NewOAuth2ProviderSettingsResource() resource.Resource { + return &OAuth2ProviderSettingsResource{} +} + +func (r *OAuth2ProviderSettingsResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_oauth2_provider_settings" +} + +func (r *OAuth2ProviderSettingsResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + MarkdownDescription: `Deployment-wide OAuth2 provider settings. + +This setting is a deployment-wide singleton, so this resource can only be +declared once. Declaring it more than once is not an error: whichever block +applies last silently wins. + +~> **Warning** +When adopting a deployment where this setting has already been configured out +of band (via ` + "`coder oauth2-provider dcr enable`" + ` or the deployment settings UI), run +` + "`terraform import`" + ` **before** your first ` + "`terraform apply`" + `. There is no prior state +for Terraform to diff against on a first apply, so the live value is overwritten +with your configured one without appearing as a change in the plan. A plan-time +warning is raised if this would disable Dynamic Client Registration where it is +currently enabled, but a warning does not block the apply. + +~> **Warning** +` + "`terraform destroy`" + ` resets ` + "`dynamic_client_registration_enabled`" + ` to ` + "`false`" + `, the +deployment default. The API has no delete operation for this setting, so the +value cannot be returned to a "never configured" state. + +-> Managing this setting is entirely optional: omit the resource to leave +Dynamic Client Registration alone. To read the current value without taking +ownership of it, use the ` + "`coderd_oauth2_provider_settings`" + ` data source instead. + +~> **Warning** +This resource is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. +`, + Attributes: map[string]schema.Attribute{ + "dynamic_client_registration_enabled": schema.BoolAttribute{ + Required: true, + MarkdownDescription: "Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) " + + "is enabled for the deployment. When disabled, `POST /oauth2/register` is rejected and the " + + "`registration_endpoint` is omitted from the authorization server metadata document.", + }, + }, + } +} + +func (r *OAuth2ProviderSettingsResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + // Prevent panic if the provider has not been configured. + if req.ProviderData == nil { + return + } + + data, ok := req.ProviderData.(*CoderdProviderData) + + if !ok { + resp.Diagnostics.AddError( + "Unable to configure provider data", + fmt.Sprintf("Expected *CoderdProviderData, got: %T. Please report this issue to the provider developers.", req.ProviderData), + ) + + return + } + + r.CoderdProviderData = data +} + +func (r *OAuth2ProviderSettingsResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + // Read Terraform prior state data into the model + var data OAuth2ProviderSettingsResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + settings, err := r.Client.OAuth2ProviderSettings(ctx) + if err != nil { + // Deliberately not treated as "resource deleted": this setting is a + // deployment singleton that always exists on a supported deployment, + // so a 404 means the endpoint is missing, not the resource. + resp.Diagnostics.Append(oauth2ProviderSettingsDiag("read", err)...) + return + } + + data.DynamicClientRegistrationEnabled = types.BoolValue(dcrEnabledOrDefault(settings)) + + // Save updated data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +func (r *OAuth2ProviderSettingsResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + // Read Terraform plan data into the model + var data OAuth2ProviderSettingsResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "creating oauth2 provider settings", map[string]any{ + "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + }) + + // Create and Update use a shared implementation: the underlying API is a + // single idempotent PUT with no separate create semantics. + resp.Diagnostics.Append(r.patch(ctx, data.DynamicClientRegistrationEnabled.ValueBool())...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "successfully created oauth2 provider settings", map[string]any{ + "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + }) + + // Save data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +func (r *OAuth2ProviderSettingsResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + // Read Terraform plan data into the model + var data OAuth2ProviderSettingsResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "updating oauth2 provider settings", map[string]any{ + "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + }) + + // Create and Update use a shared implementation + resp.Diagnostics.Append(r.patch(ctx, data.DynamicClientRegistrationEnabled.ValueBool())...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "successfully updated oauth2 provider settings", map[string]any{ + "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + }) + + // Save updated data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) +} + +func (r *OAuth2ProviderSettingsResource) patch(ctx context.Context, dcrEnabled bool) diag.Diagnostics { + var diags diag.Diagnostics + // Always send a non-nil pointer. The API treats an omitted field as "leave + // the current value alone", which is the right default for a partial + // update but wrong for this resource: it owns the value outright, so + // omitting it would make Create/Update no-ops and, worse, turn Delete's + // reset-to-default into a silent no-op. + _, err := r.Client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{ + DynamicClientRegistrationEnabled: &dcrEnabled, + }) + if err != nil { + diags.Append(oauth2ProviderSettingsDiag("update", err)...) + } + return diags +} + +func (r *OAuth2ProviderSettingsResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + tflog.Trace(ctx, "deleting oauth2 provider settings", map[string]any{}) + + // There is no DELETE endpoint for this setting: it is a `site_configs` + // upsert. Reset to the deployment default instead, so `terraform destroy` + // leaves the deployment in a well-defined state rather than stranding the + // last-applied value with no Terraform record of it. + // + // If this fails, the appended error keeps the resource in state, so a + // subsequent `terraform destroy` retries rather than the admin wrongly + // believing DCR was reset. + resp.Diagnostics.Append(r.patch(ctx, oauth2ProviderSettingsDefaultDCR)...) + if resp.Diagnostics.HasError() { + return + } + + tflog.Trace(ctx, "successfully deleted oauth2 provider settings", map[string]any{}) +} + +// ModifyPlan warns, at plan time, when a first apply is about to disable +// Dynamic Client Registration on a deployment where it is currently enabled. +// +// That is the one direction worth flagging. A create has no prior state to +// diff against, so Terraform renders it as a plain "will be created" with no +// hint that a live value is being discarded. Surfacing it during plan is what +// gives the admin a chance to `terraform import` instead, while nothing has +// been changed yet. +// +// Deliberately not symmetric: a live `false` is indistinguishable from "never +// configured", because the server coalesces both, so warning whenever the live +// value merely differs from the plan would fire on every greenfield apply +// (live `false`, config `true`) for no benefit. +func (r *OAuth2ProviderSettingsResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + // A destroy plan has a null plan. Nothing to advise on. + if req.Plan.Raw.IsNull() { + return + } + // Only a genuine create reaches the no-prior-state case this warns about. + // `terraform import` populates state without ever running Create(), so + // this correctly stays quiet on the first plan after an import. + if !req.State.Raw.IsNull() { + return + } + // Configure() has not run during the validate walk. + if r.CoderdProviderData == nil { + return + } + + var data OAuth2ProviderSettingsResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + // A Required attribute can still be unknown when it comes from an input + // variable or a module output. Defer rather than guess. + if data.DynamicClientRegistrationEnabled.IsUnknown() || data.DynamicClientRegistrationEnabled.IsNull() { + return + } + if data.DynamicClientRegistrationEnabled.ValueBool() { + // Enabling never discards an out-of-band value. + return + } + + settings, err := r.Client.OAuth2ProviderSettings(ctx) + if err != nil { + // Best-effort advisory only. The error is not swallowed: Create() + // makes the same call for real moments later and reports it there, + // with the right wording for the operation that actually failed. + // Raising it here instead would turn every unreachable-or-forbidden + // deployment into a confusing plan failure about a warning. + tflog.Debug(ctx, "skipping oauth2 provider settings plan-time check", map[string]any{ + "error": err.Error(), + }) + return + } + if !dcrEnabledOrDefault(settings) { + return + } + + resp.Diagnostics.AddAttributeWarning( + pathDynamicClientRegistrationEnabled, + "Overwriting an out-of-band value", + "`dynamic_client_registration_enabled` is currently `true` on this deployment, and applying will set it "+ + "to `false`. Terraform has no prior state for this resource, so this change is not shown as a diff.\n\n"+ + "If you meant to adopt the deployment's existing value rather than overwrite it, run "+ + "`terraform import coderd_oauth2_provider_settings. oauth2_provider_settings` first.", + ) +} + +func (r *OAuth2ProviderSettingsResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + // No identifying attribute exists to extract from req.ID: this resource is + // a deployment-wide singleton and Read() takes no parameters. Terraform + // calls Read() immediately after this to populate + // dynamic_client_registration_enabled from the live API; the import ID + // itself is required by Terraform's CLI syntax but otherwise unused. + // + // The framework requires at least one attribute be set for the import to + // produce a non-null state object for Read() to overwrite. + resp.Diagnostics.Append(resp.State.Set(ctx, OAuth2ProviderSettingsResourceModel{ + DynamicClientRegistrationEnabled: types.BoolValue(oauth2ProviderSettingsDefaultDCR), + })...) +} + +// dcrEnabledOrDefault reads the DCR flag out of a settings response. +// +// `codersdk.OAuth2ProviderSettings.DynamicClientRegistrationEnabled` is a +// pointer so a PUT can omit it to mean "leave this alone". On a GET the field +// is documented to always come back non-nil, so nil here means the deployment +// answered with something the contract says it never should. Fall back to the +// same default the server itself applies to a never-configured deployment +// rather than panicking or inventing a value. +func dcrEnabledOrDefault(settings codersdk.OAuth2ProviderSettings) bool { + if settings.DynamicClientRegistrationEnabled == nil { + return oauth2ProviderSettingsDefaultDCR + } + return *settings.DynamicClientRegistrationEnabled +} + +// oauth2ProviderSettingsDiag converts a codersdk error from the OAuth2 +// provider settings endpoint into a diagnostic. Both the resource and the data +// source route through here so a deployment that predates the endpoint +// produces the same actionable message either way. +func oauth2ProviderSettingsDiag(action string, err error) diag.Diagnostics { + var diags diag.Diagnostics + + // Not isNotFound: that helper also maps a 400 "must be an existing uuid or + // username" to not-found, which is meaningless for a parameterless + // endpoint and would mislabel an unrelated bad request as a version + // problem. + var sdkErr *codersdk.Error + if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { + diags.AddError( + "Unsupported Coder Version", + fmt.Sprintf("Unable to %s OAuth2 provider settings: the deployment returned 404 for %s. "+ + "This endpoint requires Coder version %s or later; upgrade the deployment, or remove "+ + "`coderd_oauth2_provider_settings` from your configuration. Original error: %s", + action, "/api/v2/oauth2-provider/settings", oauth2ProviderSettingsMinVersion, err), + ) + return diags + } + + diags.AddError("Client Error", fmt.Sprintf("unable to %s OAuth2 provider settings, got error: %s", action, err)) + return diags +} diff --git a/internal/provider/oauth2_provider_settings_resource_test.go b/internal/provider/oauth2_provider_settings_resource_test.go new file mode 100644 index 0000000..a6c6bda --- /dev/null +++ b/internal/provider/oauth2_provider_settings_resource_test.go @@ -0,0 +1,771 @@ +package provider + +import ( + "fmt" + "net/http" + "net/url" + "os" + "regexp" + "testing" + + "github.com/coder/coder/v2/codersdk" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Test case coverage for the OAuth2 provider settings resource. The TC numbers +// refer to the ENG-3083 proposal, section 5. +// +// Two of the proposal's cases are deliberately not implemented here, because +// neither is observable from inside this repository's test process: +// +// - TC10 (provider version predates the feature) is a property of which +// compiled provider binary Terraform loads. Every test in this package +// links the current package, so the resource type always exists. The +// failure mode ("Invalid resource type") is produced by Terraform core +// before any provider code runs, not by anything this repo can regress. +// - TC12 (Coder upgraded by a `helm_release` in the same apply, no +// `depends_on`) depends on Terraform's graph walk across a third-party +// provider. Its documented outcome is that an unordered apply collapses +// into TC11, and TC11 is covered below: the 404 surfaces as one clean, +// actionable error rather than a hang or a corrupted state entry. +// +// Both are documented on the resource's schema instead. + +const oauth2SettingsResourceAddr = "coderd_oauth2_provider_settings.test" + +func oauth2SettingsProviderBlock(url string) string { + return fmt.Sprintf(` +provider "coderd" { + url = %q + token = "test-token" +} +`, url) +} + +// oauth2SettingsConfig renders a provider block plus a single settings +// resource. +func oauth2SettingsConfig(url string, dcrEnabled bool) string { + return oauth2SettingsProviderBlock(url) + fmt.Sprintf(` +resource "coderd_oauth2_provider_settings" "test" { + dynamic_client_registration_enabled = %t +} +`, dcrEnabled) +} + +// TestDCREnabledOrDefault covers the nil-handling forced by +// `DynamicClientRegistrationEnabled` being a `*bool`. The pointer exists so a +// PUT can omit the field to mean "leave this alone"; on a GET the field is +// documented to always come back non-nil, so the nil branch is defensive +// against a contract violation rather than an expected response. +func TestDCREnabledOrDefault(t *testing.T) { + t.Parallel() + + enabled, disabled := true, false + + for _, tc := range []struct { + name string + in *bool + want bool + }{ + {name: "True", in: &enabled, want: true}, + {name: "False", in: &disabled, want: false}, + { + // Never expected from a GET. Falls back to the same default the + // server applies to a never-configured deployment, rather than + // panicking or inventing a value. + name: "NilFallsBackToDeploymentDefault", + in: nil, + want: oauth2ProviderSettingsDefaultDCR, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := dcrEnabledOrDefault(codersdk.OAuth2ProviderSettings{ + DynamicClientRegistrationEnabled: tc.in, + }) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestOAuth2ProviderSettingsModifyPlan covers the plan-time advisory that +// makes TC3's overwrite visible before anything is applied. A plain unit test +// rather than a TestAcc one: the assertion is on the diagnostics ModifyPlan +// emits, which needs no Terraform binary, and the test framework at this +// version has no way to match a warning in plan output. +func TestOAuth2ProviderSettingsModifyPlan(t *testing.T) { + t.Parallel() + + objType := tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "dynamic_client_registration_enabled": tftypes.Bool, + }, + } + // boolObject builds a state/plan value. A nil `v` yields a null object, + // which is how the framework signals "no prior state" (a create) and "no + // plan" (a destroy). + boolObject := func(v any) tftypes.Value { + if v == nil { + return tftypes.NewValue(objType, nil) + } + return tftypes.NewValue(objType, map[string]tftypes.Value{ + "dynamic_client_registration_enabled": tftypes.NewValue(tftypes.Bool, v), + }) + } + nullObject := boolObject(nil) + + for _, tc := range []struct { + name string + // live is the deployment's current value. + live bool + // lookupStatus, when non-zero, makes the settings GET fail. + lookupStatus int + plan tftypes.Value + state tftypes.Value + wantWarnings int + wantGets int + }{ + { + // The TC3 scenario: DCR is on, the first apply would turn it off. + name: "WarnsWhenFirstApplyWouldDisableLiveEnabled", + live: true, + plan: boolObject(false), + state: nullObject, + wantWarnings: 1, + wantGets: 1, + }, + { + // A never-configured deployment reads back as false, so warning + // here would fire on every greenfield apply. + name: "SilentWhenEnablingOnGreenfieldDeployment", + live: false, + plan: boolObject(true), + state: nullObject, + wantWarnings: 0, + wantGets: 0, // enabling is never destructive, so no lookup + }, + { + name: "SilentWhenPlanAlreadyMatchesLiveValue", + live: true, + plan: boolObject(true), + state: nullObject, + wantWarnings: 0, + wantGets: 0, + }, + { + name: "SilentWhenDisablingAlreadyDisabled", + live: false, + plan: boolObject(false), + state: nullObject, + wantWarnings: 0, + wantGets: 1, // looked up, found nothing worth reporting + }, + { + // Not a create. An update already renders a real diff, and this is + // also the first plan after `terraform import`. + name: "SilentWhenPriorStateExists", + live: true, + plan: boolObject(false), + state: boolObject(true), + wantWarnings: 0, + wantGets: 0, + }, + { + name: "SilentOnDestroyPlan", + live: true, + plan: nullObject, + state: boolObject(true), + wantWarnings: 0, + wantGets: 0, + }, + { + // A Required attribute is still unknown when it comes from an + // input variable or module output. + name: "SilentWhenPlannedValueUnknown", + live: true, + plan: boolObject(tftypes.UnknownValue), + state: nullObject, + wantWarnings: 0, + wantGets: 0, + }, + { + // Best-effort: a failed lookup must not turn into a plan error. + // Create() makes the same call and reports it properly. + name: "SilentWhenLookupFails", + live: true, + lookupStatus: http.StatusForbidden, + plan: boolObject(false), + state: nullObject, + wantWarnings: 0, + wantGets: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + f := newFakeCoderd(t) + f.SetDCREnabled(tc.live) + if tc.lookupStatus != 0 { + f.SetGetStatus(tc.lookupStatus) + } + + serverURL, err := url.Parse(f.URL) + require.NoError(t, err) + client := codersdk.New(serverURL) + client.SetSessionToken("test-token") + + r := &OAuth2ProviderSettingsResource{ + CoderdProviderData: &CoderdProviderData{Client: client}, + } + + schemaResp := &fwresource.SchemaResponse{} + r.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) + require.Empty(t, schemaResp.Diagnostics) + s := schemaResp.Schema + + resp := &fwresource.ModifyPlanResponse{Plan: tfsdk.Plan{Schema: s, Raw: tc.plan}} + r.ModifyPlan(ctx, fwresource.ModifyPlanRequest{ + Config: tfsdk.Config{Schema: s, Raw: tc.plan}, + Plan: tfsdk.Plan{Schema: s, Raw: tc.plan}, + State: tfsdk.State{Schema: s, Raw: tc.state}, + }, resp) + + assert.Empty(t, resp.Diagnostics.Errors(), "a plan-time advisory must never fail the plan") + assert.Len(t, resp.Diagnostics.Warnings(), tc.wantWarnings) + assert.Equal(t, tc.wantGets, f.SettingsRequestCount(http.MethodGet)) + }) + } +} + +func TestAccOAuth2ProviderSettingsResource(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + + // TC2 — First Create against a never-configured deployment. + t.Run("TC2_CreateAgainstNeverConfigured", func(t *testing.T) { + f := newFakeCoderd(t) + // Live value: never configured, which the server coalesces to false. + require.False(t, f.DCREnabled()) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(oauth2SettingsResourceAddr, plancheck.ResourceActionCreate), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsResourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), + }, + Check: func(*terraform.State) error { + assert.True(t, f.DCREnabled(), "live value should have been enabled") + return nil + }, + }, + }, + }) + }) + + // TC3 — First Create against a deployment already configured out of band, + // without importing first. This asserts the footgun exists, so it stays a + // documented guarantee rather than an accident: the live value is flipped + // with no diff and, because there is no prior state, no way for Terraform + // to render it as a change. + // + // The overwrite itself still happens; ModifyPlan makes it *visible* at plan + // time rather than preventing it (see + // TestOAuth2ProviderSettingsModifyPlan/WarnsWhenFirstApplyWouldDisableLiveEnabled). + // An apply is not blocked by a warning, so the guarantee asserted here is + // unchanged. + t.Run("TC3_CreateOverwritesOutOfBandValue", func(t *testing.T) { + f := newFakeCoderd(t) + // Enabled previously via `coder oauth2-provider dcr enable` or the UI. + f.SetDCREnabled(true) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, false), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // A Create, not an Update: there is no prior state + // to diff the live `true` against. + plancheck.ExpectResourceAction(oauth2SettingsResourceAddr, plancheck.ResourceActionCreate), + }, + }, + Check: func(*terraform.State) error { + assert.False(t, f.DCREnabled(), "live value should have been silently overwritten to false") + puts := f.SettingsRequests() + require.NotEmpty(t, puts) + last := puts[len(puts)-1] + assert.Equal(t, http.MethodPut, last.Method) + assert.False(t, last.DCREnabled) + return nil + }, + }, + }, + }) + }) + + // TC4 — Declared but the required attribute is omitted. Terraform's own + // schema validation rejects this before Configure() runs, so no request is + // ever made. + t.Run("TC4_RequiredAttributeOmitted", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsProviderBlock(f.URL) + ` +resource "coderd_oauth2_provider_settings" "test" { +} +`, + ExpectError: regexp.MustCompile(`The argument "dynamic_client_registration_enabled" is required`), + }, + }, + }) + + assert.Empty(t, f.SettingsRequests(), "config validation must fail before any API call") + }) + + // TC5 — Drift detection: live value changed out of band after a successful + // apply. + t.Run("TC5_DriftDetection", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + }, + { + // Another actor flips the value via the CLI or API. + PreConfig: func() { f.SetDCREnabled(false) }, + Config: oauth2SettingsConfig(f.URL, true), + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + }, + }) + }) + + // TC6 — No-diff refresh when the never-configured default matches config. + t.Run("TC6_NoDiffWhenDefaultMatches", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, false), + }, + { + // The harness fails a PlanOnly step on any diff unless + // ExpectNonEmptyPlan is set, so this step passing is the + // assertion. + Config: oauth2SettingsConfig(f.URL, false), + PlanOnly: true, + }, + }, + }) + }) + + // TC7 — Update flips the value with no resource replacement. + t.Run("TC7_UpdateInPlace", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + }, + { + Config: oauth2SettingsConfig(f.URL, false), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(oauth2SettingsResourceAddr, plancheck.ResourceActionUpdate), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsResourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(false)), + }, + Check: func(*terraform.State) error { + assert.False(t, f.DCREnabled()) + return nil + }, + }, + }, + }) + }) + + // TC8 — Destroy resets to the documented default. + t.Run("TC8_DestroyResetsToDefault", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + CheckDestroy: func(*terraform.State) error { + // The API has no DELETE verb, so Delete() resets to the + // deployment default via the same PUT. + if f.DCREnabled() { + return fmt.Errorf("expected destroy to reset the live value to false") + } + reqs := f.SettingsRequests() + last := reqs[len(reqs)-1] + if last.Method != http.MethodPut || last.DCREnabled { + return fmt.Errorf("expected the final request to be a PUT of false, got %+v", last) + } + // The field must be explicitly present. The API reads an + // omitted field as "leave the current value alone", so a + // regression that stopped sending it would turn this reset + // into a silent no-op while every other assertion still + // passed. + if !last.DCRSent { + return fmt.Errorf("destroy must send dynamic_client_registration_enabled explicitly, not omit it") + } + return nil + }, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + Check: func(*terraform.State) error { + assert.True(t, f.DCREnabled()) + return nil + }, + }, + }, + }) + }) + + // TC9 — Destroy fails cleanly on a transient error: the failure is + // surfaced and the resource stays in state, so a retry is possible. + t.Run("TC9_DestroyFailsCleanly", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + }, + { + PreConfig: func() { f.SetPutStatus(http.StatusInternalServerError) }, + Config: oauth2SettingsConfig(f.URL, true), + Destroy: true, + ExpectError: regexp.MustCompile(`unable to update OAuth2 provider settings`), + }, + { + // The live value was never reset, and the resource is + // still in state: this step is a no-op apply, which the + // harness only accepts if both are true. (A dropped state + // entry would re-Create; a reset live value would diff.) + PreConfig: func() { f.SetPutStatus(0) }, + Config: oauth2SettingsConfig(f.URL, true), + Check: func(*terraform.State) error { + assert.True(t, f.DCREnabled(), "a failed destroy must not have changed the live value") + return nil + }, + }, + }, + }) + }) + + // TC11 — New provider, old Coderd (pre-#27316): the endpoint 404s and the + // provider must name the minimum version rather than leaking a decode + // failure. + t.Run("TC11_OldCoderd", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetGetStatus(http.StatusNotFound) + f.SetPutStatus(http.StatusNotFound) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + ExpectError: regexp.MustCompile(`Unsupported Coder Version`), + }, + }, + }) + }) + + // TC13 — Token holding neither read nor update on the deployment config + // (e.g. a plain Member). The provider makes no client-side authorization + // decision; it surfaces whatever the API returned. + t.Run("TC13_Forbidden", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetGetStatus(http.StatusForbidden) + f.SetPutStatus(http.StatusForbidden) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + ExpectError: regexp.MustCompile(`(?s)unable to update OAuth2 provider settings.*Forbidden`), + }, + }, + }) + }) + + // TC14 — Concurrent external mutation mid-apply. There is no optimistic + // concurrency control (no ETag or version field), so the last PUT to land + // wins and Terraform cannot detect the collision. Documented limitation, + // asserted so it stays documented. + t.Run("TC14_ConcurrentExternalMutationLastWriteWins", func(t *testing.T) { + f := newFakeCoderd(t) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + }, + { + PreConfig: func() { + // Another actor writes `true` in the window between + // this apply's refresh and its PUT. + f.SetBeforeGet(nil) + f.mu.Lock() + f.beforePut = func(f *fakeCoderd) { f.SetDCREnabled(true) } + f.mu.Unlock() + }, + Config: oauth2SettingsConfig(f.URL, false), + Check: func(*terraform.State) error { + // Terraform's write landed last, so it wins. No error, + // no detection. + assert.False(t, f.DCREnabled()) + return nil + }, + }, + }, + }) + }) + + // TC15 — Two resource blocks targeting the same singleton. This is not an + // error: the API has no "already exists" concept, so the blocks silently + // fight, leaving a permanent diff. + t.Run("TC15_TwoBlocksSameSingleton", func(t *testing.T) { + f := newFakeCoderd(t) + + cfg := oauth2SettingsProviderBlock(f.URL) + ` +resource "coderd_oauth2_provider_settings" "a" { + dynamic_client_registration_enabled = true +} + +resource "coderd_oauth2_provider_settings" "b" { + dynamic_client_registration_enabled = false +} +` + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: cfg, + // Whichever block applied second overwrote the other, so + // the loser refreshes to a value that disagrees with its + // config, forever. + ExpectNonEmptyPlan: true, + Check: func(*terraform.State) error { + assert.Equal(t, 2, f.SettingsRequestCount(http.MethodPut), + "both blocks should have written, with no collision error") + return nil + }, + }, + }, + }) + }) + + // TC21 — Adopting a deployment via `terraform import`, the safe + // alternative to TC3's bare first apply. + t.Run("TC21_ImportAdoptsLiveValue", func(t *testing.T) { + f := newFakeCoderd(t) + // Already enabled out of band, the same starting point as TC3. + f.SetDCREnabled(true) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + // The admin's desired value differs from what is live. + Config: oauth2SettingsConfig(f.URL, false), + ResourceName: oauth2SettingsResourceAddr, + ImportState: true, + ImportStateId: "oauth2_provider_settings", + ImportStatePersist: true, + ImportStateCheck: func(states []*terraform.InstanceState) error { + if len(states) != 1 { + return fmt.Errorf("expected 1 imported instance, got %d", len(states)) + } + if got := states[0].Attributes["dynamic_client_registration_enabled"]; got != "true" { + return fmt.Errorf("expected the imported state to hold the live value true, got %q", got) + } + return nil + }, + Check: func(*terraform.State) error { + // The core guarantee: import reads, it never writes. + assert.Zero(t, f.SettingsRequestCount(http.MethodPut), + "import must never call PutOAuth2ProviderSettings") + assert.True(t, f.DCREnabled(), "import must not disturb the live value") + return nil + }, + }, + { + // Now the admin gets an honest, reviewable diff instead of + // TC3's silent overwrite. + Config: oauth2SettingsConfig(f.URL, false), + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + }, + }) + }) + + // TC23 — Auditor-role token: the GET succeeds, the PUT is forbidden. A + // genuinely distinct outcome from both TC2 (all succeed) and TC13 (all + // fail), on one token. + t.Run("TC23_ReadOkWriteForbidden", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetDCREnabled(true) + f.SetPutStatus(http.StatusForbidden) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + // Import exercises the read path alone, so this step + // succeeding proves the GET is permitted. + Config: oauth2SettingsConfig(f.URL, false), + ResourceName: oauth2SettingsResourceAddr, + ImportState: true, + ImportStateId: "oauth2_provider_settings", + ImportStatePersist: true, + ImportStateCheck: func(states []*terraform.InstanceState) error { + if got := states[0].Attributes["dynamic_client_registration_enabled"]; got != "true" { + return fmt.Errorf("expected read to succeed and yield true, got %q", got) + } + return nil + }, + }, + { + // The write path, on the same token, is refused. + Config: oauth2SettingsConfig(f.URL, false), + ExpectError: regexp.MustCompile(`(?s)unable to update OAuth2 provider settings.*Forbidden`), + }, + { + // Restore write access purely so the harness can destroy. + PreConfig: func() { f.SetPutStatus(0) }, + Config: oauth2SettingsConfig(f.URL, false), + }, + }, + }) + }) +} + +// TestAccOAuth2ProviderSettingsNotDeclared covers the proposal's 5.1 group: +// declining to manage this setting is always safe, and costs nothing but +// omitting the block. +func TestAccOAuth2ProviderSettingsNotDeclared(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + + // unrelatedConfig stands in for a configuration that manages other things + // and has no interest in OAuth2 or DCR. `terraform_data` is a Terraform + // builtin, so this exercises an apply that touches no coderd resource at + // all. + unrelatedConfig := func(url, value string) string { + return oauth2SettingsProviderBlock(url) + fmt.Sprintf(` +resource "terraform_data" "unrelated" { + input = %q +} +`, value) + } + + // TC1 — The resource is simply not declared. Nothing plans, nothing + // applies, nothing is called. + // TC20 — The same, while making an unrelated change: shipping a new + // resource type does not retroactively require existing configs to declare + // it, so the `Required` attribute is never evaluated. + // TC22 — The same again, with the live value already `true`, to isolate + // where TC3's risk actually comes from: declaring the resource with an + // unverified value, not declining to manage it. + for _, tc := range []struct { + name string + liveDCR bool + }{ + {name: "TC1_TC20_NotDeclared", liveDCR: false}, + {name: "TC22_NotDeclaredLiveTrue", liveDCR: true}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newFakeCoderd(t) + f.SetDCREnabled(tc.liveDCR) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + {Config: unrelatedConfig(f.URL, "before")}, + {Config: unrelatedConfig(f.URL, "after")}, + }, + }) + + assert.Empty(t, f.SettingsRequests(), + "an apply that does not declare the resource must never touch the settings endpoint") + assert.Equal(t, tc.liveDCR, f.DCREnabled(), "the live value must be undisturbed") + }) + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 441de81..1a70cc8 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -236,6 +236,7 @@ func (p *CoderdProvider) Resources(ctx context.Context) []func() resource.Resour NewAIProviderResource, NewAgentsModelResource, NewDefaultAgentsModelResource, + NewOAuth2ProviderSettingsResource, } } @@ -245,6 +246,7 @@ func (p *CoderdProvider) DataSources(ctx context.Context) []func() datasource.Da NewUserDataSource, NewOrganizationDataSource, NewTemplateDataSource, + NewOAuth2ProviderSettingsDataSource, } } From abcb900014643963d865d557e55aeab3ff65085b Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 27 Jul 2026 14:47:16 -0700 Subject: [PATCH 3/9] fix(coderd_oauth2_provider_settings): name the oauth2 experiment on a 403 coderd gates the whole `/api/v2/oauth2-provider` route behind the `oauth2` experiment via `httpmw.RequireExperimentWithDevBypass`. With the experiment off, route middleware answers 403 before any handler runs, so the DCR setting can be neither read nor changed. That is the correct behaviour, and it is enforced server-side; the problem was how the provider reported it. A bare "Client Error" is indistinguishable at a glance from the RBAC denial a Member or Auditor token produces, and neither the schema docs nor the examples mentioned the requirement. The experiment is off by default and is not covered by `--experiments='*'` (`ExperimentsSafe` lists only `ExperimentMinimumImplicitMember`), so a stock release deployment hits this on a first apply. `oauth2ProviderSettingsDiag` now separates the experiment gate from an RBAC denial and raises "OAuth2 Experiment Not Enabled", naming `CODER_EXPERIMENTS=oauth2` / `--experiments=oauth2` while preserving coderd's own message. This mirrors `isWorkspaceSharingExperimentOff` in `organization_resource.go`, which solves the same problem for the workspace-sharing experiment. Discriminating on message text is unavoidable because coderd reuses 403 for both refusals, but it degrades safely: a reworded upstream message falls back to the generic "Client Error" this branch replaced. A 404 still takes the "Unsupported Coder Version" branch, so the two remedies -- upgrade Coder versus change its configuration -- stay distinguishable. The path cannot be reached through `scripts/develop.sh`, because development builds bypass the experiment check outright, so coverage is a fake serving the real 403 body verbatim plus a table test for the discriminator that concentrates on the inputs which must *not* match: a false positive would relabel every genuine permission error as "enable the experiment". --- docs/data-sources/oauth2_provider_settings.md | 9 ++ docs/resources/oauth2_provider_settings.md | 12 ++ .../data-source.tf | 4 + .../resource.tf | 7 + .../oauth2_provider_settings_data_source.go | 3 + ...uth2_provider_settings_data_source_test.go | 23 +++ .../oauth2_provider_settings_fake_test.go | 46 +++++- .../oauth2_provider_settings_resource.go | 55 +++++++ .../oauth2_provider_settings_resource_test.go | 140 ++++++++++++++++++ 9 files changed, 296 insertions(+), 3 deletions(-) diff --git a/docs/data-sources/oauth2_provider_settings.md b/docs/data-sources/oauth2_provider_settings.md index a209838..7bfcd72 100644 --- a/docs/data-sources/oauth2_provider_settings.md +++ b/docs/data-sources/oauth2_provider_settings.md @@ -14,6 +14,8 @@ description: |- read this data source but cannot own the equivalent resource. ~> Warning This data source is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. + ~> Warning + The deployment must have the oauth2 experiment enabled (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2). The experiment gates the entire /api/v2/oauth2-provider route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. --- # coderd_oauth2_provider_settings (Data Source) @@ -33,6 +35,9 @@ read this data source but cannot own the equivalent resource. ~> **Warning** This data source is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. +~> **Warning** +The deployment must have the `oauth2` experiment enabled (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`). The experiment gates the entire `/api/v2/oauth2-provider` route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. + ## Example Usage ```terraform @@ -43,6 +48,10 @@ This data source is only compatible with Coder version [2.35.0](https://github.c // // This only issues a GET, so it also works with tokens that can read but not // write the deployment configuration, such as an Auditor token. +// +// It does still require the deployment's `oauth2` experiment to be enabled: the +// experiment gates the entire /api/v2/oauth2-provider route, so a read-only +// token does not get you around it. data "coderd_oauth2_provider_settings" "current" {} // Because Dynamic Client Registration is disabled, provision a static OAuth2 diff --git a/docs/resources/oauth2_provider_settings.md b/docs/resources/oauth2_provider_settings.md index 46a2908..cc7644c 100644 --- a/docs/resources/oauth2_provider_settings.md +++ b/docs/resources/oauth2_provider_settings.md @@ -24,6 +24,8 @@ description: |- ownership of it, use the coderd_oauth2_provider_settings data source instead. ~> Warning This resource is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. + ~> Warning + The deployment must have the oauth2 experiment enabled (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2). It is off by default, and --experiments='*' does not enable it, so it must be named explicitly. Without it, /api/v2/oauth2-provider/settings returns 403 and this resource cannot manage the setting. Development builds of Coder bypass the check, so a -devel deployment works either way — which makes this easy to miss until you apply against a release. --- # coderd_oauth2_provider_settings (Resource) @@ -55,9 +57,19 @@ ownership of it, use the `coderd_oauth2_provider_settings` data source instead. ~> **Warning** This resource is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. +~> **Warning** +The deployment must have the `oauth2` experiment enabled (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`). It is **off by default**, and `--experiments='*'` does **not** enable it, so it must be named explicitly. Without it, `/api/v2/oauth2-provider/settings` returns `403` and this resource cannot manage the setting. Development builds of Coder bypass the check, so a `-devel` deployment works either way — which makes this easy to miss until you apply against a release. + ## Example Usage ```terraform +// Prerequisite: the deployment must have the `oauth2` experiment enabled +// (CODER_EXPERIMENTS=oauth2, or --experiments=oauth2). It is off by default and +// is NOT included in `--experiments='*'`, so it has to be named explicitly. +// Without it the API returns 403 and this resource cannot manage the setting. +// (Development builds of Coder bypass the check, so a `-devel` deployment works +// either way -- which makes this easy to miss until you hit a real release.) +// // Important note: this setting is a deployment-wide singleton, so you can only // have one resource of this type! Declaring it twice is not an error, the // blocks will silently overwrite each other on every apply. diff --git a/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf b/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf index 7ab8bff..333eb2b 100644 --- a/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf +++ b/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf @@ -5,6 +5,10 @@ // // This only issues a GET, so it also works with tokens that can read but not // write the deployment configuration, such as an Auditor token. +// +// It does still require the deployment's `oauth2` experiment to be enabled: the +// experiment gates the entire /api/v2/oauth2-provider route, so a read-only +// token does not get you around it. data "coderd_oauth2_provider_settings" "current" {} // Because Dynamic Client Registration is disabled, provision a static OAuth2 diff --git a/examples/resources/coderd_oauth2_provider_settings/resource.tf b/examples/resources/coderd_oauth2_provider_settings/resource.tf index 1a297b8..cf4b863 100644 --- a/examples/resources/coderd_oauth2_provider_settings/resource.tf +++ b/examples/resources/coderd_oauth2_provider_settings/resource.tf @@ -1,3 +1,10 @@ +// Prerequisite: the deployment must have the `oauth2` experiment enabled +// (CODER_EXPERIMENTS=oauth2, or --experiments=oauth2). It is off by default and +// is NOT included in `--experiments='*'`, so it has to be named explicitly. +// Without it the API returns 403 and this resource cannot manage the setting. +// (Development builds of Coder bypass the check, so a `-devel` deployment works +// either way -- which makes this easy to miss until you hit a real release.) +// // Important note: this setting is a deployment-wide singleton, so you can only // have one resource of this type! Declaring it twice is not an error, the // blocks will silently overwrite each other on every apply. diff --git a/internal/provider/oauth2_provider_settings_data_source.go b/internal/provider/oauth2_provider_settings_data_source.go index 6ce31aa..223c0ea 100644 --- a/internal/provider/oauth2_provider_settings_data_source.go +++ b/internal/provider/oauth2_provider_settings_data_source.go @@ -46,6 +46,9 @@ read this data source but cannot own the equivalent resource. ~> **Warning** This data source is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. + +~> **Warning** +The deployment must have the ` + "`" + oauth2ProviderSettingsExperiment + "`" + ` experiment enabled (` + "`CODER_EXPERIMENTS=" + oauth2ProviderSettingsExperiment + "`" + ` or ` + "`--experiments=" + oauth2ProviderSettingsExperiment + "`" + `). The experiment gates the entire ` + "`/api/v2/oauth2-provider`" + ` route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. `, Attributes: map[string]schema.Attribute{ diff --git a/internal/provider/oauth2_provider_settings_data_source_test.go b/internal/provider/oauth2_provider_settings_data_source_test.go index c1bd5bd..e67de41 100644 --- a/internal/provider/oauth2_provider_settings_data_source_test.go +++ b/internal/provider/oauth2_provider_settings_data_source_test.go @@ -86,6 +86,29 @@ func TestAccOAuth2ProviderSettingsDataSource(t *testing.T) { }) }) + // Beyond the proposal: the `oauth2` experiment is disabled. The experiment + // gates the whole /api/v2/oauth2-provider route, not just its write path, so + // read-only access is no escape hatch — this is the one respect in which the + // data source is *not* more permissive than the resource. Worth asserting + // separately from TC17, since a reader who can work around an RBAC denial by + // switching to the data source cannot work around this. + t.Run("ExperimentDisabled", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetExperimentOff() + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsDataSourceConfig(f.URL), + ExpectError: regexp.MustCompile(`(?s)OAuth2 Experiment Not Enabled.*CODER_EXPERIMENTS=oauth2`), + }, + }, + }) + }) + // TC18 — Data source against an old Coderd: the same 404-derived error as // TC11, since both route through Client.OAuth2ProviderSettings. t.Run("TC18_OldCoderd", func(t *testing.T) { diff --git a/internal/provider/oauth2_provider_settings_fake_test.go b/internal/provider/oauth2_provider_settings_fake_test.go index 89ab1ac..a96c5d8 100644 --- a/internal/provider/oauth2_provider_settings_fake_test.go +++ b/internal/provider/oauth2_provider_settings_fake_test.go @@ -39,6 +39,12 @@ type fakeCoderd struct { getStatus int putStatus int + // errMessage, when non-empty, replaces the generic message statusMessage + // derives from the status code. Needed because coderd reuses 403 for two + // unrelated refusals -- an RBAC denial and the `oauth2` experiment gate -- + // and only the message distinguishes them. + errMessage string + // beforeGet runs before each settings GET is answered, and beforeSettingsPut // before each PUT. Both are used to simulate another actor mutating the // deployment mid-apply. @@ -101,11 +107,11 @@ func (f *fakeCoderd) handleSettingsGet(w http.ResponseWriter) { } f.mu.Lock() - status, enabled := f.getStatus, f.dcrEnabled + status, enabled, msg := f.getStatus, f.dcrEnabled, f.errMessage f.mu.Unlock() if status != 0 { - writeJSON(w, status, codersdk.Response{Message: statusMessage(status)}) + writeJSON(w, status, codersdk.Response{Message: errorMessage(status, msg)}) return } // A GET always answers with a non-nil pointer, matching the documented @@ -117,6 +123,7 @@ func (f *fakeCoderd) handleSettingsPut(w http.ResponseWriter, r *http.Request) { f.mu.Lock() hook := f.beforePut status := f.putStatus + msg := f.errMessage f.mu.Unlock() if hook != nil { hook(f) @@ -145,12 +152,33 @@ func (f *fakeCoderd) handleSettingsPut(w http.ResponseWriter, r *http.Request) { f.mu.Unlock() if status != 0 { - writeJSON(w, status, codersdk.Response{Message: statusMessage(status)}) + writeJSON(w, status, codersdk.Response{Message: errorMessage(status, msg)}) return } writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{DynamicClientRegistrationEnabled: &resolved}) } +// oauth2ExperimentOffMessage is the message coderd's `httpmw.RequireExperiment` +// returns alongside its 403 when the OAuth2 experiment is disabled, copied +// verbatim from the single-experiment branch of that middleware: +// +// fmt.Sprintf("%s functionality requires enabling the '%s' experiment.", +// experiment.DisplayName(), experiment) +// +// Reproduced exactly because isOAuth2ExperimentOff discriminates on this text -- +// a paraphrase here would let the test pass while the real message stopped +// matching. +const oauth2ExperimentOffMessage = "OAuth2 Provider Functionality functionality requires enabling the 'oauth2' experiment." + +// errorMessage returns the override when set, otherwise the generic message for +// the status code. +func errorMessage(status int, override string) string { + if override != "" { + return override + } + return statusMessage(status) +} + // statusMessage mirrors the shape of the message coderd itself returns, so the // provider's error text is assertable. func statusMessage(status int) string { @@ -198,6 +226,18 @@ func (f *fakeCoderd) SetPutStatus(status int) { f.putStatus = status } +// SetExperimentOff makes both settings verbs fail the way a deployment without +// the `oauth2` experiment does: a 403 whose message names the experiment. This +// is the only practical way to reach that path -- `scripts/develop.sh` cannot, +// because development builds bypass the experiment check entirely. +func (f *fakeCoderd) SetExperimentOff() { + f.mu.Lock() + defer f.mu.Unlock() + f.getStatus = http.StatusForbidden + f.putStatus = http.StatusForbidden + f.errMessage = oauth2ExperimentOffMessage +} + func (f *fakeCoderd) SetBeforeGet(hook func(f *fakeCoderd)) { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index db129e1..2d9070b 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/coder/coder/v2/codersdk" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -26,6 +27,13 @@ var _ resource.ResourceWithModifyPlan = &OAuth2ProviderSettingsResource{} // actionable message instead of a bare "not found". const oauth2ProviderSettingsMinVersion = "2.35.0" +// oauth2ProviderSettingsExperiment is the coderd experiment gating the whole +// `/api/v2/oauth2-provider` route, settings included. While it is unset, route +// middleware refuses every request with a 403 before any handler runs, so the +// setting can be neither read nor changed. It is off by default and is not +// covered by `--experiments='*'`, so this is the state of a stock deployment. +const oauth2ProviderSettingsExperiment = "oauth2" + // oauth2ProviderSettingsDefaultDCR is the deployment default for // `dynamic_client_registration_enabled`. A never-configured deployment reads // back as `false`, and `Delete` restores this value because the API has no @@ -81,6 +89,9 @@ ownership of it, use the ` + "`coderd_oauth2_provider_settings`" + ` data source ~> **Warning** This resource is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. + +~> **Warning** +The deployment must have the ` + "`" + oauth2ProviderSettingsExperiment + "`" + ` experiment enabled (` + "`CODER_EXPERIMENTS=" + oauth2ProviderSettingsExperiment + "`" + ` or ` + "`--experiments=" + oauth2ProviderSettingsExperiment + "`" + `). It is **off by default**, and ` + "`--experiments='*'`" + ` does **not** enable it, so it must be named explicitly. Without it, ` + "`/api/v2/oauth2-provider/settings`" + ` returns ` + "`403`" + ` and this resource cannot manage the setting. Development builds of Coder bypass the check, so a ` + "`-devel`" + ` deployment works either way — which makes this easy to miss until you apply against a release. `, Attributes: map[string]schema.Attribute{ "dynamic_client_registration_enabled": schema.BoolAttribute{ @@ -346,6 +357,50 @@ func oauth2ProviderSettingsDiag(action string, err error) diag.Diagnostics { return diags } + if isOAuth2ExperimentOff(err) { + diags.AddError( + "OAuth2 Experiment Not Enabled", + fmt.Sprintf("Unable to %s OAuth2 provider settings: the deployment returned 403 because the `%s` "+ + "experiment is not enabled. `%s` is gated behind that experiment, so the setting can be "+ + "neither read nor changed while it is off. Enable it on the Coder deployment "+ + "(`CODER_EXPERIMENTS=%s`, or `--experiments=%s`) and restart it, or remove "+ + "`coderd_oauth2_provider_settings` from your configuration. Original error: %s", + action, + oauth2ProviderSettingsExperiment, + "/api/v2/oauth2-provider/settings", + oauth2ProviderSettingsExperiment, + oauth2ProviderSettingsExperiment, + err), + ) + return diags + } + diags.AddError("Client Error", fmt.Sprintf("unable to %s OAuth2 provider settings, got error: %s", action, err)) return diags } + +// isOAuth2ExperimentOff reports whether err is coderd's `httpmw.RequireExperiment` +// refusal for the OAuth2 experiment rather than an RBAC denial. Both arrive as a +// 403, and only the message distinguishes them: the experiment gate names the +// experiment, while an RBAC denial is the generic "Forbidden." +// +// Mirrors isWorkspaceSharingExperimentOff, which solves the same problem for the +// workspace-sharing experiment. Matching on message text is unavoidable — coderd +// reuses 403 for both — but it degrades safely: a reworded message falls through +// to the generic "Client Error" that this branch replaced, which still surfaces +// coderd's own text. +func isOAuth2ExperimentOff(err error) bool { + var sdkErr *codersdk.Error + if !errors.As(err, &sdkErr) { + return false + } + if sdkErr.StatusCode() != http.StatusForbidden { + return false + } + // Requiring both substrings covers `RequireExperiment`'s single-experiment + // message ("... requires enabling the 'oauth2' experiment.") and its + // multi-experiment form ("... the following experiments: oauth2, ..."). + // An RBAC denial carries neither. + return strings.Contains(sdkErr.Message, oauth2ProviderSettingsExperiment) && + strings.Contains(sdkErr.Message, "experiment") +} diff --git a/internal/provider/oauth2_provider_settings_resource_test.go b/internal/provider/oauth2_provider_settings_resource_test.go index a6c6bda..61859be 100644 --- a/internal/provider/oauth2_provider_settings_resource_test.go +++ b/internal/provider/oauth2_provider_settings_resource_test.go @@ -1,6 +1,7 @@ package provider import ( + "errors" "fmt" "net/http" "net/url" @@ -98,6 +99,115 @@ func TestDCREnabledOrDefault(t *testing.T) { } } +// TestIsOAuth2ExperimentOff covers the discriminator between coderd's two +// unrelated 403s: an RBAC denial and the `oauth2` experiment gate. Only the +// message separates them, so this is the test that pins the matching rule. +// +// Worth having as a table rather than only via the framework tests below, +// because the interesting cases are the ones that must NOT match — a false +// positive here would relabel every genuine permission error as "enable the +// experiment", sending an admin to change deployment config when the real +// problem is their token. +func TestIsOAuth2ExperimentOff(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + err error + want bool + }{ + { + // The real message, verbatim from httpmw.RequireExperiment. + name: "ExperimentGate", + err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: oauth2ExperimentOffMessage}), + want: true, + }, + { + // RequireExperiment's other branch, used when a route requires + // several experiments at once. The settings route requires only + // one today, so this guards against a future change upstream. + name: "ExperimentGateMultiExperimentForm", + err: codersdk.NewError(http.StatusForbidden, codersdk.Response{ + Message: "This functionality requires enabling the following experiments: oauth2, mcp-server-http", + }), + want: true, + }, + { + // The RBAC denial from TC13/TC17/TC23. Same status code, and it + // must stay distinguishable. + name: "RBACDenialIsNotTheExperimentGate", + err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "Forbidden."}), + want: false, + }, + { + // Mentions the experiment name but is not an experiment refusal. + // Both substrings are required precisely to exclude this. + name: "ForbiddenMentioningOAuth2ButNotExperiments", + err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "OAuth2 app not found."}), + want: false, + }, + { + // An old deployment. Must stay on the version-hint branch. + name: "NotFoundIsAVersionProblem", + err: codersdk.NewError(http.StatusNotFound, codersdk.Response{Message: "Not Found."}), + want: false, + }, + { + // Right words, wrong status: the gate always answers 403. + name: "NonForbiddenStatusWithExperimentWording", + err: codersdk.NewError(http.StatusInternalServerError, codersdk.Response{ + Message: "requires enabling the 'oauth2' experiment.", + }), + want: false, + }, + { + name: "NotAnSDKError", + err: errors.New("dial tcp: connection refused"), + want: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, isOAuth2ExperimentOff(tc.err)) + }) + } +} + +// TestOAuth2ProviderSettingsExperimentDiag asserts the diagnostic the +// experiment gate produces, including that it does not collide with the two +// neighbouring branches of oauth2ProviderSettingsDiag. +func TestOAuth2ProviderSettingsExperimentDiag(t *testing.T) { + t.Parallel() + + experimentDiags := oauth2ProviderSettingsDiag("update", + codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: oauth2ExperimentOffMessage})) + require.Len(t, experimentDiags, 1) + assert.Equal(t, "OAuth2 Experiment Not Enabled", experimentDiags[0].Summary()) + // The remedy must be actionable: name the experiment and how to set it. + assert.Contains(t, experimentDiags[0].Detail(), "CODER_EXPERIMENTS=oauth2") + assert.Contains(t, experimentDiags[0].Detail(), "--experiments=oauth2") + // coderd's own text is preserved rather than swallowed. + assert.Contains(t, experimentDiags[0].Detail(), oauth2ExperimentOffMessage) + + // A 404 must still be the version hint, not the experiment hint. These two + // are the failure modes most easily confused: both mean "this deployment + // cannot do it", but one is fixed by upgrading Coder and the other by + // changing its configuration. + versionDiags := oauth2ProviderSettingsDiag("read", + codersdk.NewError(http.StatusNotFound, codersdk.Response{Message: "Not Found."})) + require.Len(t, versionDiags, 1) + assert.Equal(t, "Unsupported Coder Version", versionDiags[0].Summary()) + assert.NotContains(t, versionDiags[0].Detail(), "experiment") + + // An RBAC 403 must stay the generic client error, so it is not misreported + // as a deployment-configuration problem. + rbacDiags := oauth2ProviderSettingsDiag("update", + codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "Forbidden."})) + require.Len(t, rbacDiags, 1) + assert.Equal(t, "Client Error", rbacDiags[0].Summary()) + assert.NotContains(t, rbacDiags[0].Detail(), "experiment") +} + // TestOAuth2ProviderSettingsModifyPlan covers the plan-time advisory that // makes TC3's overwrite visible before anything is applied. A plain unit test // rather than a TestAcc one: the assertion is on the diagnostics ModifyPlan @@ -550,6 +660,36 @@ resource "coderd_oauth2_provider_settings" "test" { }) }) + // Beyond the proposal: the deployment has the `oauth2` experiment disabled, + // which is the default for a stock release deployment. Route middleware + // refuses both verbs with a 403 before any handler runs, so the setting + // cannot be changed — the behaviour we want — and the admin needs to be told + // which knob to turn, not handed the same message a bad token produces. + // + // Not reachable through `scripts/develop.sh`: development builds bypass the + // experiment check (`RequireExperimentWithDevBypass`), so a fake is the only + // way to exercise this short of a real release binary. + t.Run("ExperimentDisabled", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetExperimentOff() + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + ExpectError: regexp.MustCompile(`(?s)OAuth2 Experiment Not Enabled.*CODER_EXPERIMENTS=oauth2`), + }, + }, + }) + + // The refusal must be total: nothing was written, so a deployment with + // the experiment off cannot end up half-configured. + assert.False(t, f.DCREnabled(), "a refused apply must not have changed the live value") + }) + // TC14 — Concurrent external mutation mid-apply. There is no optimistic // concurrency control (no ETag or version field), so the last PUT to land // wins and Terraform cannot detect the collision. Documented limitation, From 07735ed7d00de30405bb1c86109818a603e525a1 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 28 Jul 2026 20:22:40 -0700 Subject: [PATCH 4/9] chore(deps): bump coder/coder to the merged DCR toggle Replace the temporary pin on coder/coder#27316's branch head (569a0eb3, `coder-eng-3056-dcr-flag`) with the squash commit that landed it on main, fbac6024. `codersdk.OAuth2ProviderSettings` now comes from main rather than from an unmerged PR, so this branch no longer depends on unreviewed code. The SDK contract is unchanged from the pinned branch head: `DynamicClientRegistrationEnabled *bool` with a `dynamic_client_registration_enabled,omitempty` tag, and `GET`/`PUT` on `/api/v2/oauth2-provider/settings`. The `feat!` marker on the upstream commit denotes a server-side behavior change -- DCR now defaults to disabled -- which matches the default this resource already resets to on destroy. Transitively bumps aws-sdk-go-v2, terraform-provider-coder, klauspost/compress, and valyala/fasthttp. --- go.mod | 10 +++++----- go.sum | 28 ++++++++++++---------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index df63b54..ad9c9e4 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.5 require ( cdr.dev/slog/v3 v3.1.0 - github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112 + github.com/coder/coder/v2 v2.34.0-rc.0.0.20260728235933-fbac602456c9 github.com/coder/retry v1.5.1 github.com/coder/serpent v0.15.0 github.com/coder/websocket v1.8.15 @@ -59,7 +59,7 @@ require ( github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect github.com/apparentlymart/go-textseg/v17 v17.0.1 // indirect github.com/armon/go-radix v1.0.1-0.20221118154546-54df44f2176c // indirect - github.com/aws/aws-sdk-go-v2 v1.42.1 // indirect + github.com/aws/aws-sdk-go-v2 v1.43.0 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymerick/douceur v0.2.0 // indirect github.com/bahlo/generic-list-go v0.2.0 // indirect @@ -75,7 +75,7 @@ require ( github.com/clipperhouse/uax29/v2 v2.6.0 // indirect github.com/cloudflare/circl v1.6.3 // indirect github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 // indirect - github.com/coder/terraform-provider-coder/v2 v2.18.0 // indirect + github.com/coder/terraform-provider-coder/v2 v2.19.0 // indirect github.com/containerd/errdefs v1.0.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect github.com/containerd/log v0.1.0 // indirect @@ -123,7 +123,7 @@ require ( github.com/imdario/mergo v0.3.16 // indirect github.com/invopop/jsonschema v0.14.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/klauspost/compress v1.19.0 // indirect + github.com/klauspost/compress v1.19.1 // indirect github.com/klauspost/cpuid/v2 v2.3.0 // indirect github.com/linkdata/deadlock v0.5.5 // indirect github.com/lucasb-eyer/go-colorful v1.4.0 // indirect @@ -187,7 +187,7 @@ require ( github.com/tklauser/go-sysconf v0.3.16 // indirect github.com/tklauser/numcpus v0.11.0 // indirect github.com/trailofbits/go-mutexasserts v0.0.0-20250514102930-c1f3d2e37561 // indirect - github.com/valyala/fasthttp v1.72.0 // indirect + github.com/valyala/fasthttp v1.73.0 // indirect github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect diff --git a/go.sum b/go.sum index 98485db..8e3b230 100644 --- a/go.sum +++ b/go.sum @@ -80,8 +80,8 @@ github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpH github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo= github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= -github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= -github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= +github.com/andybalholm/brotli v1.2.2 h1:HzTuoo2ErYQqf5qvcJInB8uvqSVxRttzkFexPWtnceM= +github.com/andybalholm/brotli v1.2.2/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY= github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4= @@ -89,8 +89,8 @@ github.com/apparentlymart/go-textseg/v17 v17.0.1 h1:bpMXRgQ5cEoRNuQke1a80/Nl6w3G github.com/apparentlymart/go-textseg/v17 v17.0.1/go.mod h1:fa8X4jgGeevslICIY6LcdjkSecWnXmYd9Lk34z/VxZs= github.com/armon/go-radix v1.0.1-0.20221118154546-54df44f2176c h1:651/eoCRnQ7YtSjAnSzRucrJz+3iGEFt+ysraELS81M= github.com/armon/go-radix v1.0.1-0.20221118154546-54df44f2176c/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/aws/aws-sdk-go-v2 v1.42.1 h1:9eOTgu1z/dVtYpNZ3/8/XbbaX0x/BqE3HUzAzs6K0ek= -github.com/aws/aws-sdk-go-v2 v1.42.1/go.mod h1:5pKeft2eJj+gElQ38Jqg4ibCqh+/AK33/0X3hip7IjM= +github.com/aws/aws-sdk-go-v2 v1.43.0 h1:fharf/WhbRAVZ1du0QL7roNFxZ6T/sWr+4Ni617bwSI= +github.com/aws/aws-sdk-go-v2 v1.43.0/go.mod h1:5pKeft2eJj+gElQ38Jqg4ibCqh+/AK33/0X3hip7IjM= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= @@ -136,18 +136,16 @@ github.com/clipperhouse/uax29/v2 v2.6.0 h1:z0cDbUV+aPASdFb2/ndFnS9ts/WNXgTNNGFoK github.com/clipperhouse/uax29/v2 v2.6.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260722122238-15be31e185b2 h1:WzI+01BOTkYee0l3h4uhD5K1/uBt5pVI8qGkyMCbpS4= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260722122238-15be31e185b2/go.mod h1:m+W7ywCko4nTGYYEo3vjp3RV6KuN80fiSui0nKCqbuI= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112 h1:gS3dUhwbomtRL/98vSZ3WXbagvCM4Nkl7ayG1173+TY= -github.com/coder/coder/v2 v2.34.0-rc.0.0.20260727150003-569a0eb34112/go.mod h1:3ehot6+0lHQvdktT7D08k/vTAa0LaLeht8azzsjkRbw= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260728235933-fbac602456c9 h1:RkSBeaPdzNiPBaXKA20ipdp3uWal6zsr4m/3T1xbKRw= +github.com/coder/coder/v2 v2.34.0-rc.0.0.20260728235933-fbac602456c9/go.mod h1:uSNvF2qSMuK/+wJuzVlvbJlFCrq658I+nlPYmgAXz+A= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= github.com/coder/serpent v0.15.0 h1:jobR7DnPsxzEMD0cRiailwlY+4v6HAPS/8emIgBpaIU= github.com/coder/serpent v0.15.0/go.mod h1:7OIvFBYMd+OqarMy5einBl8AtRr8LliopVU7pyrwucY= -github.com/coder/terraform-provider-coder/v2 v2.18.0 h1:b60ixwf7pVPuiL0GkHZf+1mVj94/HZhCNpsfjAK34mI= -github.com/coder/terraform-provider-coder/v2 v2.18.0/go.mod h1:Yowo7rLIWw3OOhWSY7LjB57kld3xiFEcUbSI04cnRpU= +github.com/coder/terraform-provider-coder/v2 v2.19.0 h1:ShiwZiUZ3wyUt4+ccrdWlUKu+24aDgzMLD6Yf6xKtAw= +github.com/coder/terraform-provider-coder/v2 v2.19.0/go.mod h1:MWTw/k+oQxySR4/wZIunBxTlFva8hH6t2hLikTpyomU= github.com/coder/websocket v1.8.15 h1:6B2JPeOGlpff2Uz6vOEH1Vzpi0iUz20A+lPVhPHtNUA= github.com/coder/websocket v1.8.15/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= @@ -318,8 +316,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM= -github.com/klauspost/compress v1.19.0 h1:sXLILfc9jV2QYWkzFOPWStmcUVH2RHEB1JCdY2oVvCQ= -github.com/klauspost/compress v1.19.0/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/klauspost/compress v1.19.1 h1:VsB4HPswih7mmZ8WleSFQ75c/Ui1M4trX5oAsJnhSlk= +github.com/klauspost/compress v1.19.1/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzhA9Y= github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= @@ -514,8 +512,8 @@ github.com/trailofbits/go-mutexasserts v0.0.0-20250514102930-c1f3d2e37561 h1:qqa github.com/trailofbits/go-mutexasserts v0.0.0-20250514102930-c1f3d2e37561/go.mod h1:GA3+Mq3kt3tYAfM0WZCu7ofy+GW9PuGysHfhr+6JX7s= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/valyala/fasthttp v1.72.0 h1:R7kYdoWhn1ye1fVpP+cDHDJwYm3NkwLliwgzJ/Abg7M= -github.com/valyala/fasthttp v1.72.0/go.mod h1:zsbLTYqcpIktdQytlVBwIjY9La5d6bs990nBxWg8efk= +github.com/valyala/fasthttp v1.73.0 h1:ocTOORnBWtJ+P8t/6wAjdkchMzdfHmWx2VD/DPbgZ7s= +github.com/valyala/fasthttp v1.73.0/go.mod h1:EtXQDHaR+5P18p8wqDRFpUhxr108Ga9mXvVJXHRrN2k= github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaUXK79GlxNBwueZn0xI= github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= @@ -743,8 +741,6 @@ google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 h1: google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7/go.mod h1:KqHwBx2upmfa1XSi1WuRvC+2VGCLtooKkfmyvRbUmqA= google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800 h1:qEHAMpSaUhtD0p3NbEEI83HwNGFxEwaSJ1G9PLnCBZE= google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.82.0 h1:vguDnZUPjE26w09A63VoxZPnvPjB5Riyc0mkXPFmAIU= -google.golang.org/grpc v1.82.0/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= From 1b55aac21aa9f693eb072599d82cc923ce9fa0eb Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 28 Jul 2026 21:37:34 -0700 Subject: [PATCH 5/9] feat(coderd_oauth2_provider_settings): drop the data source Remove the read-only `coderd_oauth2_provider_settings` data source, per review. Declaring it alongside the resource in one configuration produces a deterministic stale read: a data source with a fully-known config and no `depends_on` resolves during the plan phase, while the resource's PUT happens during apply, so any plan-time consumer of the data source sees the pre-apply value and lags one apply behind. The reviewed example fed that value to a `count`, which is the worst case. `count` is structural rather than a value, so it must be settled at plan time and is never re-evaluated during apply. Without `depends_on` the count is known-but-stale and the run silently produces a contradictory end state; with `depends_on` it becomes unknown and the configuration fails to plan at all. There is no correct spelling of that example. The data source's stated safety invariant -- that it only ever issues a GET -- rules out write/write conflicts but says nothing about read/write ordering, which is why the existing tests all passed. Every data source test exercised it in isolation. Removing rather than documenting around it: the read-without-owning use case is real but thin, and it can return later behind an enforced "never declare alongside the resource" rule rather than advisory prose. Removes the data source, its tests, its example, and its generated doc, and drops the resource schema's pointer to it. The fake test server and `oauth2ProviderSettingsDiag` stay: both are still used by the resource. --- docs/data-sources/oauth2_provider_settings.md | 76 ------- docs/resources/oauth2_provider_settings.md | 8 +- .../data-source.tf | 25 --- .../oauth2_provider_settings_data_source.go | 106 ---------- ...uth2_provider_settings_data_source_test.go | 185 ------------------ .../oauth2_provider_settings_resource.go | 10 +- .../oauth2_provider_settings_resource_test.go | 2 +- internal/provider/provider.go | 1 - 8 files changed, 10 insertions(+), 403 deletions(-) delete mode 100644 docs/data-sources/oauth2_provider_settings.md delete mode 100644 examples/data-sources/coderd_oauth2_provider_settings/data-source.tf delete mode 100644 internal/provider/oauth2_provider_settings_data_source.go delete mode 100644 internal/provider/oauth2_provider_settings_data_source_test.go diff --git a/docs/data-sources/oauth2_provider_settings.md b/docs/data-sources/oauth2_provider_settings.md deleted file mode 100644 index 7bfcd72..0000000 --- a/docs/data-sources/oauth2_provider_settings.md +++ /dev/null @@ -1,76 +0,0 @@ ---- -# generated by https://github.com/hashicorp/terraform-plugin-docs -page_title: "coderd_oauth2_provider_settings Data Source - terraform-provider-coderd" -subcategory: "" -description: |- - The deployment-wide OAuth2 provider settings. - Use this data source to read the current settings without taking ownership of - them. Because the settings are a deployment-wide singleton, only one Terraform - configuration can own the coderd_oauth2_provider_settings resource; any other - configuration that needs the value should read it here instead. - -> This data source only issues a GET, so it works with any token holding - read access on the deployment configuration, which is a broader set of roles - than the resource's write path requires. An Auditor token, for example, can - read this data source but cannot own the equivalent resource. - ~> Warning - This data source is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. - ~> Warning - The deployment must have the oauth2 experiment enabled (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2). The experiment gates the entire /api/v2/oauth2-provider route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. ---- - -# coderd_oauth2_provider_settings (Data Source) - -The deployment-wide OAuth2 provider settings. - -Use this data source to read the current settings without taking ownership of -them. Because the settings are a deployment-wide singleton, only one Terraform -configuration can own the `coderd_oauth2_provider_settings` *resource*; any other -configuration that needs the value should read it here instead. - --> This data source only issues a `GET`, so it works with any token holding -read access on the deployment configuration, which is a broader set of roles -than the resource's write path requires. An Auditor token, for example, can -read this data source but cannot own the equivalent resource. - -~> **Warning** -This data source is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. - -~> **Warning** -The deployment must have the `oauth2` experiment enabled (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`). The experiment gates the entire `/api/v2/oauth2-provider` route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. - -## Example Usage - -```terraform -// Read the deployment's OAuth2 provider settings without taking ownership of -// them. Use this when another configuration (or nobody at all) owns the -// `coderd_oauth2_provider_settings` resource, since the setting is a -// deployment-wide singleton and only one configuration can manage it. -// -// This only issues a GET, so it also works with tokens that can read but not -// write the deployment configuration, such as an Auditor token. -// -// It does still require the deployment's `oauth2` experiment to be enabled: the -// experiment gates the entire /api/v2/oauth2-provider route, so a read-only -// token does not get you around it. -data "coderd_oauth2_provider_settings" "current" {} - -// Because Dynamic Client Registration is disabled, provision a static OAuth2 -// client instead, for MCP clients that cannot register themselves. -resource "coderd_group" "mcp_static_clients" { - count = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled ? 0 : 1 - - name = "mcp-static-clients" - organization_id = data.coderd_organization.default.id -} - -output "dynamic_client_registration_enabled" { - value = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled -} -``` - - -## Schema - -### Read-Only - -- `dynamic_client_registration_enabled` (Boolean) Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) is enabled for the deployment. A deployment that has never configured this setting reads back as `false`. diff --git a/docs/resources/oauth2_provider_settings.md b/docs/resources/oauth2_provider_settings.md index cc7644c..94bf35c 100644 --- a/docs/resources/oauth2_provider_settings.md +++ b/docs/resources/oauth2_provider_settings.md @@ -20,8 +20,8 @@ description: |- deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. -> Managing this setting is entirely optional: omit the resource to leave - Dynamic Client Registration alone. To read the current value without taking - ownership of it, use the coderd_oauth2_provider_settings data source instead. + Dynamic Client Registration alone. Because the setting is a deployment-wide + singleton, only one Terraform configuration should declare this resource. ~> Warning This resource is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. ~> Warning @@ -51,8 +51,8 @@ deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. -> Managing this setting is entirely optional: omit the resource to leave -Dynamic Client Registration alone. To read the current value without taking -ownership of it, use the `coderd_oauth2_provider_settings` data source instead. +Dynamic Client Registration alone. Because the setting is a deployment-wide +singleton, only one Terraform configuration should declare this resource. ~> **Warning** This resource is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. diff --git a/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf b/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf deleted file mode 100644 index 333eb2b..0000000 --- a/examples/data-sources/coderd_oauth2_provider_settings/data-source.tf +++ /dev/null @@ -1,25 +0,0 @@ -// Read the deployment's OAuth2 provider settings without taking ownership of -// them. Use this when another configuration (or nobody at all) owns the -// `coderd_oauth2_provider_settings` resource, since the setting is a -// deployment-wide singleton and only one configuration can manage it. -// -// This only issues a GET, so it also works with tokens that can read but not -// write the deployment configuration, such as an Auditor token. -// -// It does still require the deployment's `oauth2` experiment to be enabled: the -// experiment gates the entire /api/v2/oauth2-provider route, so a read-only -// token does not get you around it. -data "coderd_oauth2_provider_settings" "current" {} - -// Because Dynamic Client Registration is disabled, provision a static OAuth2 -// client instead, for MCP clients that cannot register themselves. -resource "coderd_group" "mcp_static_clients" { - count = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled ? 0 : 1 - - name = "mcp-static-clients" - organization_id = data.coderd_organization.default.id -} - -output "dynamic_client_registration_enabled" { - value = data.coderd_oauth2_provider_settings.current.dynamic_client_registration_enabled -} diff --git a/internal/provider/oauth2_provider_settings_data_source.go b/internal/provider/oauth2_provider_settings_data_source.go deleted file mode 100644 index 223c0ea..0000000 --- a/internal/provider/oauth2_provider_settings_data_source.go +++ /dev/null @@ -1,106 +0,0 @@ -package provider - -import ( - "context" - "fmt" - - "github.com/hashicorp/terraform-plugin-framework/datasource" - "github.com/hashicorp/terraform-plugin-framework/datasource/schema" - "github.com/hashicorp/terraform-plugin-framework/types" -) - -// Ensure provider defined types fully satisfy framework interfaces. -var _ datasource.DataSource = &OAuth2ProviderSettingsDataSource{} - -func NewOAuth2ProviderSettingsDataSource() datasource.DataSource { - return &OAuth2ProviderSettingsDataSource{} -} - -// OAuth2ProviderSettingsDataSource defines the data source implementation. -type OAuth2ProviderSettingsDataSource struct { - data *CoderdProviderData -} - -// OAuth2ProviderSettingsDataSourceModel describes the data source data model. -type OAuth2ProviderSettingsDataSourceModel struct { - DynamicClientRegistrationEnabled types.Bool `tfsdk:"dynamic_client_registration_enabled"` -} - -func (d *OAuth2ProviderSettingsDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { - resp.TypeName = req.ProviderTypeName + "_oauth2_provider_settings" -} - -func (d *OAuth2ProviderSettingsDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { - resp.Schema = schema.Schema{ - MarkdownDescription: `The deployment-wide OAuth2 provider settings. - -Use this data source to read the current settings without taking ownership of -them. Because the settings are a deployment-wide singleton, only one Terraform -configuration can own the ` + "`coderd_oauth2_provider_settings`" + ` *resource*; any other -configuration that needs the value should read it here instead. - --> This data source only issues a ` + "`GET`" + `, so it works with any token holding -read access on the deployment configuration, which is a broader set of roles -than the resource's write path requires. An Auditor token, for example, can -read this data source but cannot own the equivalent resource. - -~> **Warning** -This data source is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. - -~> **Warning** -The deployment must have the ` + "`" + oauth2ProviderSettingsExperiment + "`" + ` experiment enabled (` + "`CODER_EXPERIMENTS=" + oauth2ProviderSettingsExperiment + "`" + ` or ` + "`--experiments=" + oauth2ProviderSettingsExperiment + "`" + `). The experiment gates the entire ` + "`/api/v2/oauth2-provider`" + ` route rather than just its write path, so while it is off this data source cannot read the setting either — unlike an authorization failure, a read-only token is no way around it. -`, - - Attributes: map[string]schema.Attribute{ - "dynamic_client_registration_enabled": schema.BoolAttribute{ - Computed: true, - MarkdownDescription: "Whether OAuth2 Dynamic Client Registration ([RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591)) " + - "is enabled for the deployment. A deployment that has never configured this setting reads back as `false`.", - }, - }, - } -} - -func (d *OAuth2ProviderSettingsDataSource) Configure(ctx context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { - // Prevent panic if the provider has not been configured. - if req.ProviderData == nil { - return - } - - data, ok := req.ProviderData.(*CoderdProviderData) - - if !ok { - resp.Diagnostics.AddError( - "Unexpected Data Source Configure Type", - fmt.Sprintf("Expected *CoderdProviderData, got: %T. Please report this issue to the provider developers.", req.ProviderData), - ) - - return - } - - d.data = data -} - -func (d *OAuth2ProviderSettingsDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { - var data OAuth2ProviderSettingsDataSourceModel - - // Read Terraform configuration data into the model - resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) - if resp.Diagnostics.HasError() { - return - } - - // GET only. This data source must never call PutOAuth2ProviderSettings: - // that invariant is what makes it safe to declare alongside the resource - // without risking a write conflict. - settings, err := d.data.Client.OAuth2ProviderSettings(ctx) - if err != nil { - resp.Diagnostics.Append(oauth2ProviderSettingsDiag("read", err)...) - return - } - - data.DynamicClientRegistrationEnabled = types.BoolValue(dcrEnabledOrDefault(settings)) - - // Save data into Terraform state - resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) -} diff --git a/internal/provider/oauth2_provider_settings_data_source_test.go b/internal/provider/oauth2_provider_settings_data_source_test.go deleted file mode 100644 index e67de41..0000000 --- a/internal/provider/oauth2_provider_settings_data_source_test.go +++ /dev/null @@ -1,185 +0,0 @@ -package provider - -import ( - "net/http" - "os" - "regexp" - "testing" - - "github.com/hashicorp/terraform-plugin-testing/helper/resource" - "github.com/hashicorp/terraform-plugin-testing/knownvalue" - "github.com/hashicorp/terraform-plugin-testing/statecheck" - "github.com/hashicorp/terraform-plugin-testing/terraform" - "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" - "github.com/stretchr/testify/assert" -) - -const oauth2SettingsDataSourceAddr = "data.coderd_oauth2_provider_settings.test" - -func oauth2SettingsDataSourceConfig(url string) string { - return oauth2SettingsProviderBlock(url) + ` -data "coderd_oauth2_provider_settings" "test" {} -` -} - -// TestAccOAuth2ProviderSettingsDataSource covers the proposal's 5.7 group: the -// read-without-owning path. -func TestAccOAuth2ProviderSettingsDataSource(t *testing.T) { - t.Parallel() - if os.Getenv("TF_ACC") == "" { - t.Skip("Acceptance tests are disabled.") - } - - // TC16 — Data source only, no resource declared. The invariant that makes - // it safe to declare alongside a resource owned elsewhere is that it never - // writes, asserted here against the test double rather than claimed in - // docs. - t.Run("TC16_ReadOnlyNeverPuts", func(t *testing.T) { - f := newFakeCoderd(t) - f.SetDCREnabled(true) - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, - tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), - }, - }, - { - // Repeated applies must stay read-only too. - Config: oauth2SettingsDataSourceConfig(f.URL), - ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, - tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), - }, - }, - }, - }) - - assert.Zero(t, f.SettingsRequestCount(http.MethodPut), "the data source must never issue a PUT") - assert.NotZero(t, f.SettingsRequestCount(http.MethodGet)) - assert.True(t, f.DCREnabled(), "the live value must be untouched") - }) - - // TC17 — Data source with a token that lacks read on the deployment - // config: the 403 surfaces as an error, not a silently zero-valued - // attribute. - t.Run("TC17_Forbidden", func(t *testing.T) { - f := newFakeCoderd(t) - f.SetGetStatus(http.StatusForbidden) - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ExpectError: regexp.MustCompile(`(?s)unable to read OAuth2 provider settings.*Forbidden`), - }, - }, - }) - }) - - // Beyond the proposal: the `oauth2` experiment is disabled. The experiment - // gates the whole /api/v2/oauth2-provider route, not just its write path, so - // read-only access is no escape hatch — this is the one respect in which the - // data source is *not* more permissive than the resource. Worth asserting - // separately from TC17, since a reader who can work around an RBAC denial by - // switching to the data source cannot work around this. - t.Run("ExperimentDisabled", func(t *testing.T) { - f := newFakeCoderd(t) - f.SetExperimentOff() - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ExpectError: regexp.MustCompile(`(?s)OAuth2 Experiment Not Enabled.*CODER_EXPERIMENTS=oauth2`), - }, - }, - }) - }) - - // TC18 — Data source against an old Coderd: the same 404-derived error as - // TC11, since both route through Client.OAuth2ProviderSettings. - t.Run("TC18_OldCoderd", func(t *testing.T) { - f := newFakeCoderd(t) - f.SetGetStatus(http.StatusNotFound) - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ExpectError: regexp.MustCompile(`Unsupported Coder Version`), - }, - }, - }) - }) - - // TC19 — Fresh deployment, never configured, observed through the data - // source only. The value reaches state even though no configuration - // anywhere supplied one, because the data source's only attribute is - // Computed. - t.Run("TC19_FreshDeploymentDefaultsFalse", func(t *testing.T) { - f := newFakeCoderd(t) - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, - tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(false)), - }, - Check: func(*terraform.State) error { - assert.Zero(t, f.SettingsRequestCount(http.MethodPut)) - return nil - }, - }, - }, - }) - }) - - // TC24 — Auditor-role token: the data source resolves fine even though the - // same token could not own the equivalent resource (TC23). The data source - // only ever calls the read-gated GET, so the write-gated 403 is never - // reached. - t.Run("TC24_ReadableByTokenThatCannotWrite", func(t *testing.T) { - f := newFakeCoderd(t) - f.SetDCREnabled(true) - f.SetPutStatus(http.StatusForbidden) - - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: oauth2SettingsDataSourceConfig(f.URL), - ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue(oauth2SettingsDataSourceAddr, - tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), - }, - }, - }, - }) - - assert.Zero(t, f.SettingsRequestCount(http.MethodPut), - "the forbidden write path must never be exercised by the data source") - }) -} diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index 2d9070b..c2bbb9d 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -84,8 +84,8 @@ deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. -> Managing this setting is entirely optional: omit the resource to leave -Dynamic Client Registration alone. To read the current value without taking -ownership of it, use the ` + "`coderd_oauth2_provider_settings`" + ` data source instead. +Dynamic Client Registration alone. Because the setting is a deployment-wide +singleton, only one Terraform configuration should declare this resource. ~> **Warning** This resource is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. @@ -335,9 +335,9 @@ func dcrEnabledOrDefault(settings codersdk.OAuth2ProviderSettings) bool { } // oauth2ProviderSettingsDiag converts a codersdk error from the OAuth2 -// provider settings endpoint into a diagnostic. Both the resource and the data -// source route through here so a deployment that predates the endpoint -// produces the same actionable message either way. +// provider settings endpoint into a diagnostic. Every CRUD path routes through +// here so a deployment that predates the endpoint produces the same actionable +// message whichever operation hit it first. func oauth2ProviderSettingsDiag(action string, err error) diag.Diagnostics { var diags diag.Diagnostics diff --git a/internal/provider/oauth2_provider_settings_resource_test.go b/internal/provider/oauth2_provider_settings_resource_test.go index 61859be..a0ef28c 100644 --- a/internal/provider/oauth2_provider_settings_resource_test.go +++ b/internal/provider/oauth2_provider_settings_resource_test.go @@ -133,7 +133,7 @@ func TestIsOAuth2ExperimentOff(t *testing.T) { want: true, }, { - // The RBAC denial from TC13/TC17/TC23. Same status code, and it + // The RBAC denial from TC13/TC23. Same status code, and it // must stay distinguishable. name: "RBACDenialIsNotTheExperimentGate", err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "Forbidden."}), diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 1a70cc8..7177a46 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -246,7 +246,6 @@ func (p *CoderdProvider) DataSources(ctx context.Context) []func() datasource.Da NewUserDataSource, NewOrganizationDataSource, NewTemplateDataSource, - NewOAuth2ProviderSettingsDataSource, } } From a2b0ee66588e16bb7ff3f9a0759b2ac668363adc Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Wed, 29 Jul 2026 09:10:11 -0700 Subject: [PATCH 6/9] fix(coderd_oauth2_provider_settings): name the failing op, and build state from the PUT response Two review nits on `patch()`, taken together because both change its signature. Name the operation that failed. `patch()` passed a hard-coded "update" to `oauth2ProviderSettingsDiag`, so a failed create reported "unable to update OAuth2 provider settings" and a failed `terraform destroy` reported it too. Every write path shares one idempotent PUT, but the diagnostic should name what the user asked for: callers now pass "create", "update", and "reset". Two `ExpectError` regexes cemented the wrong wording and are updated with it. Build state from the PUT response rather than the plan. `patch()` discarded the settings the endpoint echoes back and `Create`/`Update` wrote the plan value into state. The two agree today -- the handler answers with `ptr.Ref(resolvedEnabled)` and stores exactly what it is sent -- so this is not a bug fix. It is so that if the API ever starts normalizing the value, the divergence fails loudly instead of leaving a falsehood in state to surface as drift on a later plan. Since the attribute is Required, Terraform requires final state to equal the planned value, so what this buys is detection, not tolerance. The nil branch deliberately does not use `dcrEnabledOrDefault`: that coerces an absent field to false, which here would turn a successful `true` apply into a spurious "Provider produced inconsistent result after apply". Falling back to the value just sent is the only handling that preserves the contract. Both behaviours are pinned by new tests, each verified to fail against the previous implementation -- the `dcrEnabledOrDefault` variant reproduces that exact error. The fake grows two hooks to make the response distinguishable from the request, which a real deployment never is. --- .../oauth2_provider_settings_fake_test.go | 38 ++++++++++ .../oauth2_provider_settings_resource.go | 50 ++++++++++--- .../oauth2_provider_settings_resource_test.go | 72 +++++++++++++++++-- 3 files changed, 144 insertions(+), 16 deletions(-) diff --git a/internal/provider/oauth2_provider_settings_fake_test.go b/internal/provider/oauth2_provider_settings_fake_test.go index a96c5d8..fc8958c 100644 --- a/internal/provider/oauth2_provider_settings_fake_test.go +++ b/internal/provider/oauth2_provider_settings_fake_test.go @@ -50,6 +50,20 @@ type fakeCoderd struct { // deployment mid-apply. beforeGet func(f *fakeCoderd) beforePut func(f *fakeCoderd) + + // putEcho, when non-nil, is what a successful PUT reports back regardless of + // what was stored. The real endpoint always echoes the value it just wrote, + // so this models an API that started normalizing the value -- the only way + // to observe whether the provider builds state from the response or from the + // plan, since the two are otherwise indistinguishable. + putEcho *bool + + // putEchoOmit makes a successful PUT answer with the field absent. The real + // endpoint always sets it (`ptr.Ref(resolvedEnabled)`), so this is a + // contract violation by construction -- it exists to pin down which value + // the provider falls back to, which is the difference between a clean apply + // and a spurious "inconsistent result after apply". + putEchoOmit bool } type fakeRequest struct { @@ -149,12 +163,21 @@ func (f *fakeCoderd) handleSettingsPut(w http.ResponseWriter, r *http.Request) { } } resolved := f.dcrEnabled + echo, omit := f.putEcho, f.putEchoOmit f.mu.Unlock() if status != 0 { writeJSON(w, status, codersdk.Response{Message: errorMessage(status, msg)}) return } + if omit { + // A nil pointer plus `omitempty` drops the key entirely. + writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{}) + return + } + if echo != nil { + resolved = *echo + } writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{DynamicClientRegistrationEnabled: &resolved}) } @@ -244,6 +267,21 @@ func (f *fakeCoderd) SetBeforeGet(hook func(f *fakeCoderd)) { f.beforeGet = hook } +// SetPutEcho makes a successful PUT report v back regardless of what it stored, +// standing in for an API that normalizes the value. +func (f *fakeCoderd) SetPutEcho(v bool) { + f.mu.Lock() + defer f.mu.Unlock() + f.putEcho = &v +} + +// SetPutEchoOmit makes a successful PUT answer without the field at all. +func (f *fakeCoderd) SetPutEchoOmit() { + f.mu.Lock() + defer f.mu.Unlock() + f.putEchoOmit = true +} + // SettingsRequests returns every request made to the settings endpoint, in // order. func (f *fakeCoderd) SettingsRequests() []fakeRequest { diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index c2bbb9d..4732958 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -161,13 +161,15 @@ func (r *OAuth2ProviderSettingsResource) Create(ctx context.Context, req resourc // Create and Update use a shared implementation: the underlying API is a // single idempotent PUT with no separate create semantics. - resp.Diagnostics.Append(r.patch(ctx, data.DynamicClientRegistrationEnabled.ValueBool())...) + effective, diags := r.patch(ctx, "create", data.DynamicClientRegistrationEnabled.ValueBool()) + resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } + data.DynamicClientRegistrationEnabled = types.BoolValue(effective) tflog.Trace(ctx, "successfully created oauth2 provider settings", map[string]any{ - "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + "dynamic_client_registration_enabled": effective, }) // Save data into Terraform state @@ -187,33 +189,59 @@ func (r *OAuth2ProviderSettingsResource) Update(ctx context.Context, req resourc }) // Create and Update use a shared implementation - resp.Diagnostics.Append(r.patch(ctx, data.DynamicClientRegistrationEnabled.ValueBool())...) + effective, diags := r.patch(ctx, "update", data.DynamicClientRegistrationEnabled.ValueBool()) + resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } + data.DynamicClientRegistrationEnabled = types.BoolValue(effective) tflog.Trace(ctx, "successfully updated oauth2 provider settings", map[string]any{ - "dynamic_client_registration_enabled": data.DynamicClientRegistrationEnabled.ValueBool(), + "dynamic_client_registration_enabled": effective, }) // Save updated data into Terraform state resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } -func (r *OAuth2ProviderSettingsResource) patch(ctx context.Context, dcrEnabled bool) diag.Diagnostics { +// patch writes dcrEnabled via the settings PUT and returns the value the +// deployment reports afterwards. +// +// action names the Terraform operation being performed ("create", "update", +// "reset") purely so a failure is reported with the wording of the operation +// that actually failed: every write path shares this one PUT, but "unable to +// update" on a `terraform destroy` is needlessly confusing. +func (r *OAuth2ProviderSettingsResource) patch(ctx context.Context, action string, dcrEnabled bool) (bool, diag.Diagnostics) { var diags diag.Diagnostics // Always send a non-nil pointer. The API treats an omitted field as "leave // the current value alone", which is the right default for a partial // update but wrong for this resource: it owns the value outright, so // omitting it would make Create/Update no-ops and, worse, turn Delete's // reset-to-default into a silent no-op. - _, err := r.Client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{ + updated, err := r.Client.PutOAuth2ProviderSettings(ctx, codersdk.OAuth2ProviderSettings{ DynamicClientRegistrationEnabled: &dcrEnabled, }) if err != nil { - diags.Append(oauth2ProviderSettingsDiag("update", err)...) + diags.Append(oauth2ProviderSettingsDiag(action, err)...) + return dcrEnabled, diags } - return diags + + // Prefer the value the deployment echoes back over the one just sent, so + // state records what was actually stored rather than what was asked for. + // The two are identical today -- the endpoint writes exactly what it is + // given -- so this is not a bug fix. It is so that if the API ever starts + // normalizing the value, the divergence fails loudly instead of leaving a + // falsehood in state to be discovered as drift on some later plan. + // + // Deliberately *not* dcrEnabledOrDefault: that coerces nil to false, which + // here would turn a successful `true` apply into a spurious "Provider + // produced inconsistent result after apply". The attribute is Required, so + // Terraform requires final state to equal the planned value; falling back + // to what was sent is the only nil handling that preserves that contract. + if updated.DynamicClientRegistrationEnabled == nil { + return dcrEnabled, diags + } + return *updated.DynamicClientRegistrationEnabled, diags } func (r *OAuth2ProviderSettingsResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { @@ -227,8 +255,10 @@ func (r *OAuth2ProviderSettingsResource) Delete(ctx context.Context, req resourc // If this fails, the appended error keeps the resource in state, so a // subsequent `terraform destroy` retries rather than the admin wrongly // believing DCR was reset. - resp.Diagnostics.Append(r.patch(ctx, oauth2ProviderSettingsDefaultDCR)...) - if resp.Diagnostics.HasError() { + // The returned value is discarded: the resource is leaving state, so there + // is nothing left to record it against. + if _, diags := r.patch(ctx, "reset", oauth2ProviderSettingsDefaultDCR); diags.HasError() { + resp.Diagnostics.Append(diags...) return } diff --git a/internal/provider/oauth2_provider_settings_resource_test.go b/internal/provider/oauth2_provider_settings_resource_test.go index a0ef28c..c2e8d4a 100644 --- a/internal/provider/oauth2_provider_settings_resource_test.go +++ b/internal/provider/oauth2_provider_settings_resource_test.go @@ -597,10 +597,12 @@ resource "coderd_oauth2_provider_settings" "test" { Config: oauth2SettingsConfig(f.URL, true), }, { - PreConfig: func() { f.SetPutStatus(http.StatusInternalServerError) }, - Config: oauth2SettingsConfig(f.URL, true), - Destroy: true, - ExpectError: regexp.MustCompile(`unable to update OAuth2 provider settings`), + PreConfig: func() { f.SetPutStatus(http.StatusInternalServerError) }, + Config: oauth2SettingsConfig(f.URL, true), + Destroy: true, + // "reset", not "update": every write path shares one PUT, + // but the diagnostic names the operation that failed. + ExpectError: regexp.MustCompile(`unable to reset OAuth2 provider settings`), }, { // The live value was never reset, and the resource is @@ -653,8 +655,9 @@ resource "coderd_oauth2_provider_settings" "test" { ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { - Config: oauth2SettingsConfig(f.URL, true), - ExpectError: regexp.MustCompile(`(?s)unable to update OAuth2 provider settings.*Forbidden`), + Config: oauth2SettingsConfig(f.URL, true), + // "create": this is a first apply with no prior state. + ExpectError: regexp.MustCompile(`(?s)unable to create OAuth2 provider settings.*Forbidden`), }, }, }) @@ -763,6 +766,63 @@ resource "coderd_oauth2_provider_settings" "b" { }) }) + // State is built from the value the PUT reports back, not from the plan. + // The two agree on a real deployment, so the only way to tell them apart is + // to make the fake disagree. Both cases below fail if `patch()` goes back to + // discarding the response. + t.Run("StateComesFromThePutResponse", func(t *testing.T) { + f := newFakeCoderd(t) + // The deployment stores `true` as asked but claims `false`, as an API + // that had started normalizing the value would. + f.SetPutEcho(false) + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + // The attribute is Required, so Terraform demands final + // state equal the planned value. Reporting the server's + // answer therefore surfaces the divergence as a hard error + // rather than leaving a falsehood in state to be found as + // drift on a later plan. Sourcing state from the plan + // instead would apply cleanly and hide it. + ExpectError: regexp.MustCompile(`(?s)Provider produced inconsistent result after apply`), + }, + }, + }) + }) + + // The nil branch of that same read. `dcrEnabledOrDefault` would coerce an + // absent field to `false` and turn this successful apply into the error + // asserted above; falling back to the value just sent is what keeps + // Terraform's plan-equals-state contract intact. + t.Run("AbsentPutResponseFieldFallsBackToThePlanValue", func(t *testing.T) { + f := newFakeCoderd(t) + f.SetPutEchoOmit() + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: oauth2SettingsConfig(f.URL, true), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(oauth2SettingsResourceAddr, + tfjsonpath.New("dynamic_client_registration_enabled"), knownvalue.Bool(true)), + }, + Check: func(*terraform.State) error { + assert.True(t, f.DCREnabled(), "the value sent should still have been stored") + return nil + }, + }, + }, + }) + }) + // TC21 — Adopting a deployment via `terraform import`, the safe // alternative to TC3's bare first apply. t.Run("TC21_ImportAdoptsLiveValue", func(t *testing.T) { From 952bc3c5a8ef26ae649aaedb338bf5d5746d7dbd Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Wed, 29 Jul 2026 12:43:48 -0700 Subject: [PATCH 7/9] refactor(coderd_oauth2_provider_settings): drop the experiment-gate classifier Remove `isOAuth2ExperimentOff` and the bespoke "OAuth2 Experiment Not Enabled" diagnostic, per review. coderd answers 403 for two unrelated reasons -- an RBAC denial and the `oauth2` experiment gate -- and its own message already distinguishes them: the gate names the experiment, an RBAC denial is the generic "Forbidden." The generic branch renders the `codersdk.Error`, which includes that message, so passing it through is just as actionable. What the classifier cost was a text match on wording copied from coderd's `httpmw.RequireExperiment`. That can silently degrade to a no-op when the wording changes upstream, and the test guarding it could not catch that, because the fake supplied the same copied string the matcher looked for -- the fake's own comment conceded as much. Nothing had ever observed the match against a real deployment either: the gate is unreachable on a `-devel` build, which bypasses it entirely, so every run of that path was the fake agreeing with itself. If a richer remedy is wanted, it needs a machine-readable error code from coderd rather than string matching here. Tests keep the property that matters and drop the one that could rot. `TestIsOAuth2ExperimentOff` (7 cases pinning the match) is gone. `TestOAuth2ProviderSettingsExperimentDiag` becomes `TestOAuth2ProviderSettingsDiag`, asserting that a 403 of either cause produces one generic diagnostic containing the server's message, plus the surviving 404 version hint and a non-SDK error case. The fake's verbatim message constant is replaced by `SetForbiddenWithMessage`, and both it and the acceptance subtest now use an obviously-synthetic string, so no test depends on coderd's exact wording. `docs/` is unchanged: the schema's experiment warning describes the deployment requirement, not the diagnostic, so it stays accurate. --- .../oauth2_provider_settings_fake_test.go | 37 ++-- .../oauth2_provider_settings_resource.go | 54 +----- .../oauth2_provider_settings_resource_test.go | 158 +++++++----------- 3 files changed, 83 insertions(+), 166 deletions(-) diff --git a/internal/provider/oauth2_provider_settings_fake_test.go b/internal/provider/oauth2_provider_settings_fake_test.go index fc8958c..9d959f3 100644 --- a/internal/provider/oauth2_provider_settings_fake_test.go +++ b/internal/provider/oauth2_provider_settings_fake_test.go @@ -20,8 +20,8 @@ const oauth2SettingsPath = "/api/v2/oauth2-provider/settings" // // A fake rather than `integration.StartCoder` because most of the OAuth2 // settings test matrix is about what the *provider* does with a given API -// response: that a 404 becomes a version hint, that a 403 becomes a clean -// diagnostic, that import issues no PUT at all. Those are assertions about +// response: that a 404 becomes a version hint, that a 403 passes coderd's own +// message through, that import issues no PUT at all. Those are assertions about // requests made and not made, which a real deployment cannot report. type fakeCoderd struct { *httptest.Server @@ -40,9 +40,10 @@ type fakeCoderd struct { putStatus int // errMessage, when non-empty, replaces the generic message statusMessage - // derives from the status code. Needed because coderd reuses 403 for two - // unrelated refusals -- an RBAC denial and the `oauth2` experiment gate -- - // and only the message distinguishes them. + // derives from the status code. Used to assert that whatever coderd says + // reaches the user: the provider does not classify 403s, so its two unrelated + // causes -- an RBAC denial and the `oauth2` experiment gate -- are handled + // identically and only the server's message distinguishes them. errMessage string // beforeGet runs before each settings GET is answered, and beforeSettingsPut @@ -181,18 +182,6 @@ func (f *fakeCoderd) handleSettingsPut(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, codersdk.OAuth2ProviderSettings{DynamicClientRegistrationEnabled: &resolved}) } -// oauth2ExperimentOffMessage is the message coderd's `httpmw.RequireExperiment` -// returns alongside its 403 when the OAuth2 experiment is disabled, copied -// verbatim from the single-experiment branch of that middleware: -// -// fmt.Sprintf("%s functionality requires enabling the '%s' experiment.", -// experiment.DisplayName(), experiment) -// -// Reproduced exactly because isOAuth2ExperimentOff discriminates on this text -- -// a paraphrase here would let the test pass while the real message stopped -// matching. -const oauth2ExperimentOffMessage = "OAuth2 Provider Functionality functionality requires enabling the 'oauth2' experiment." - // errorMessage returns the override when set, otherwise the generic message for // the status code. func errorMessage(status int, override string) string { @@ -249,16 +238,18 @@ func (f *fakeCoderd) SetPutStatus(status int) { f.putStatus = status } -// SetExperimentOff makes both settings verbs fail the way a deployment without -// the `oauth2` experiment does: a 403 whose message names the experiment. This -// is the only practical way to reach that path -- `scripts/develop.sh` cannot, -// because development builds bypass the experiment check entirely. -func (f *fakeCoderd) SetExperimentOff() { +// SetForbiddenWithMessage makes both settings verbs fail with a 403 carrying the +// given message. Deliberately message-agnostic: the provider does no client-side +// classification of 403s, so the property under test is only that coderd's own +// message reaches the user. Callers pass an obviously-synthetic string rather +// than a copy of coderd's wording, so nothing here can rot when that wording +// changes upstream. +func (f *fakeCoderd) SetForbiddenWithMessage(msg string) { f.mu.Lock() defer f.mu.Unlock() f.getStatus = http.StatusForbidden f.putStatus = http.StatusForbidden - f.errMessage = oauth2ExperimentOffMessage + f.errMessage = msg } func (f *fakeCoderd) SetBeforeGet(hook func(f *fakeCoderd)) { diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index 4732958..1bc2881 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net/http" - "strings" "github.com/coder/coder/v2/codersdk" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -387,50 +386,15 @@ func oauth2ProviderSettingsDiag(action string, err error) diag.Diagnostics { return diags } - if isOAuth2ExperimentOff(err) { - diags.AddError( - "OAuth2 Experiment Not Enabled", - fmt.Sprintf("Unable to %s OAuth2 provider settings: the deployment returned 403 because the `%s` "+ - "experiment is not enabled. `%s` is gated behind that experiment, so the setting can be "+ - "neither read nor changed while it is off. Enable it on the Coder deployment "+ - "(`CODER_EXPERIMENTS=%s`, or `--experiments=%s`) and restart it, or remove "+ - "`coderd_oauth2_provider_settings` from your configuration. Original error: %s", - action, - oauth2ProviderSettingsExperiment, - "/api/v2/oauth2-provider/settings", - oauth2ProviderSettingsExperiment, - oauth2ProviderSettingsExperiment, - err), - ) - return diags - } - + // Every other failure, 403s included, passes coderd's own message straight + // through. A 403 has two unrelated causes here -- an RBAC denial and the + // experiment gate -- and coderd's message already distinguishes them: the + // gate's names the experiment, an RBAC denial is the generic "Forbidden." + // Deliberately no client-side classifier for that: discriminating would mean + // matching on message text copied from coderd, which can silently rot into a + // no-op when the wording upstream changes. Passing the message through is + // just as actionable and cannot rot. If a richer remedy is wanted, it needs a + // machine-readable error code from coderd rather than string matching here. diags.AddError("Client Error", fmt.Sprintf("unable to %s OAuth2 provider settings, got error: %s", action, err)) return diags } - -// isOAuth2ExperimentOff reports whether err is coderd's `httpmw.RequireExperiment` -// refusal for the OAuth2 experiment rather than an RBAC denial. Both arrive as a -// 403, and only the message distinguishes them: the experiment gate names the -// experiment, while an RBAC denial is the generic "Forbidden." -// -// Mirrors isWorkspaceSharingExperimentOff, which solves the same problem for the -// workspace-sharing experiment. Matching on message text is unavoidable — coderd -// reuses 403 for both — but it degrades safely: a reworded message falls through -// to the generic "Client Error" that this branch replaced, which still surfaces -// coderd's own text. -func isOAuth2ExperimentOff(err error) bool { - var sdkErr *codersdk.Error - if !errors.As(err, &sdkErr) { - return false - } - if sdkErr.StatusCode() != http.StatusForbidden { - return false - } - // Requiring both substrings covers `RequireExperiment`'s single-experiment - // message ("... requires enabling the 'oauth2' experiment.") and its - // multi-experiment form ("... the following experiments: oauth2, ..."). - // An RBAC denial carries neither. - return strings.Contains(sdkErr.Message, oauth2ProviderSettingsExperiment) && - strings.Contains(sdkErr.Message, "experiment") -} diff --git a/internal/provider/oauth2_provider_settings_resource_test.go b/internal/provider/oauth2_provider_settings_resource_test.go index c2e8d4a..b616ccf 100644 --- a/internal/provider/oauth2_provider_settings_resource_test.go +++ b/internal/provider/oauth2_provider_settings_resource_test.go @@ -99,113 +99,68 @@ func TestDCREnabledOrDefault(t *testing.T) { } } -// TestIsOAuth2ExperimentOff covers the discriminator between coderd's two -// unrelated 403s: an RBAC denial and the `oauth2` experiment gate. Only the -// message separates them, so this is the test that pins the matching rule. +// TestOAuth2ProviderSettingsDiag covers the three branches of the shared +// diagnostic helper: the 404 version hint, and everything else falling through to +// a generic client error that preserves coderd's own message. // -// Worth having as a table rather than only via the framework tests below, -// because the interesting cases are the ones that must NOT match — a false -// positive here would relabel every genuine permission error as "enable the -// experiment", sending an admin to change deployment config when the real -// problem is their token. -func TestIsOAuth2ExperimentOff(t *testing.T) { +// There is deliberately no branch for the `oauth2` experiment gate. coderd +// answers 403 for two unrelated reasons -- an RBAC denial and the experiment gate +// -- and telling them apart client-side would mean matching on message text +// copied from coderd's middleware. That match can silently become a no-op when +// the wording changes upstream, and the test guarding it would keep passing, +// because the test supplies the same copied string. Passing the message through +// is just as actionable and cannot rot. A richer remedy needs a machine-readable +// error code from coderd, not string matching here. +func TestOAuth2ProviderSettingsDiag(t *testing.T) { t.Parallel() + // A 404 is the one status worth special-casing: it means the endpoint is + // absent, which is a version problem with an actionable fix, and coderd's + // own "Not Found." says nothing useful about that. + versionDiags := oauth2ProviderSettingsDiag("read", + codersdk.NewError(http.StatusNotFound, codersdk.Response{Message: "Not Found."})) + require.Len(t, versionDiags, 1) + assert.Equal(t, "Unsupported Coder Version", versionDiags[0].Summary()) + assert.Contains(t, versionDiags[0].Detail(), oauth2ProviderSettingsMinVersion) + + // Every 403 is a generic client error, whatever its cause, and coderd's + // message must survive into the detail. Both cases below assert the same + // property, which is the whole point of not classifying: the provider treats + // them identically and lets the server explain itself. for _, tc := range []struct { - name string - err error - want bool + name string + message string }{ { - // The real message, verbatim from httpmw.RequireExperiment. - name: "ExperimentGate", - err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: oauth2ExperimentOffMessage}), - want: true, - }, - { - // RequireExperiment's other branch, used when a route requires - // several experiments at once. The settings route requires only - // one today, so this guards against a future change upstream. - name: "ExperimentGateMultiExperimentForm", - err: codersdk.NewError(http.StatusForbidden, codersdk.Response{ - Message: "This functionality requires enabling the following experiments: oauth2, mcp-server-http", - }), - want: true, - }, - { - // The RBAC denial from TC13/TC23. Same status code, and it - // must stay distinguishable. - name: "RBACDenialIsNotTheExperimentGate", - err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "Forbidden."}), - want: false, - }, - { - // Mentions the experiment name but is not an experiment refusal. - // Both substrings are required precisely to exclude this. - name: "ForbiddenMentioningOAuth2ButNotExperiments", - err: codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "OAuth2 app not found."}), - want: false, + // An RBAC denial, as TC13/TC23 produce. + name: "RBACDenial", + message: "Forbidden.", }, { - // An old deployment. Must stay on the version-hint branch. - name: "NotFoundIsAVersionProblem", - err: codersdk.NewError(http.StatusNotFound, codersdk.Response{Message: "Not Found."}), - want: false, - }, - { - // Right words, wrong status: the gate always answers 403. - name: "NonForbiddenStatusWithExperimentWording", - err: codersdk.NewError(http.StatusInternalServerError, codersdk.Response{ - Message: "requires enabling the 'oauth2' experiment.", - }), - want: false, - }, - { - name: "NotAnSDKError", - err: errors.New("dial tcp: connection refused"), - want: false, + // The experiment gate. The string is deliberately synthetic rather + // than a copy of coderd's wording: nothing here depends on the exact + // text, so there is nothing to keep in sync. + name: "ExperimentGate", + message: "", }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, isOAuth2ExperimentOff(tc.err)) + diags := oauth2ProviderSettingsDiag("update", + codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: tc.message})) + require.Len(t, diags, 1) + assert.Equal(t, "Client Error", diags[0].Summary()) + assert.Contains(t, diags[0].Detail(), tc.message, + "coderd's own message must reach the user, since the provider adds no interpretation") + assert.Contains(t, diags[0].Detail(), "unable to update") }) } -} - -// TestOAuth2ProviderSettingsExperimentDiag asserts the diagnostic the -// experiment gate produces, including that it does not collide with the two -// neighbouring branches of oauth2ProviderSettingsDiag. -func TestOAuth2ProviderSettingsExperimentDiag(t *testing.T) { - t.Parallel() - experimentDiags := oauth2ProviderSettingsDiag("update", - codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: oauth2ExperimentOffMessage})) - require.Len(t, experimentDiags, 1) - assert.Equal(t, "OAuth2 Experiment Not Enabled", experimentDiags[0].Summary()) - // The remedy must be actionable: name the experiment and how to set it. - assert.Contains(t, experimentDiags[0].Detail(), "CODER_EXPERIMENTS=oauth2") - assert.Contains(t, experimentDiags[0].Detail(), "--experiments=oauth2") - // coderd's own text is preserved rather than swallowed. - assert.Contains(t, experimentDiags[0].Detail(), oauth2ExperimentOffMessage) - - // A 404 must still be the version hint, not the experiment hint. These two - // are the failure modes most easily confused: both mean "this deployment - // cannot do it", but one is fixed by upgrading Coder and the other by - // changing its configuration. - versionDiags := oauth2ProviderSettingsDiag("read", - codersdk.NewError(http.StatusNotFound, codersdk.Response{Message: "Not Found."})) - require.Len(t, versionDiags, 1) - assert.Equal(t, "Unsupported Coder Version", versionDiags[0].Summary()) - assert.NotContains(t, versionDiags[0].Detail(), "experiment") - - // An RBAC 403 must stay the generic client error, so it is not misreported - // as a deployment-configuration problem. - rbacDiags := oauth2ProviderSettingsDiag("update", - codersdk.NewError(http.StatusForbidden, codersdk.Response{Message: "Forbidden."})) - require.Len(t, rbacDiags, 1) - assert.Equal(t, "Client Error", rbacDiags[0].Summary()) - assert.NotContains(t, rbacDiags[0].Detail(), "experiment") + // A non-SDK error still produces one clean diagnostic rather than panicking. + netDiags := oauth2ProviderSettingsDiag("create", errors.New("dial tcp: connection refused")) + require.Len(t, netDiags, 1) + assert.Equal(t, "Client Error", netDiags[0].Summary()) + assert.Contains(t, netDiags[0].Detail(), "connection refused") } // TestOAuth2ProviderSettingsModifyPlan covers the plan-time advisory that @@ -665,16 +620,21 @@ resource "coderd_oauth2_provider_settings" "test" { // Beyond the proposal: the deployment has the `oauth2` experiment disabled, // which is the default for a stock release deployment. Route middleware - // refuses both verbs with a 403 before any handler runs, so the setting - // cannot be changed — the behaviour we want — and the admin needs to be told - // which knob to turn, not handed the same message a bad token produces. + // refuses both verbs with a 403 before any handler runs, so the setting cannot + // be changed — the behaviour we want — and coderd's own message names the + // experiment, so the admin is told which knob to turn without the provider + // interpreting anything. This test asserts that pass-through rather than a + // provider-authored remedy; see TestOAuth2ProviderSettingsDiag for why. // // Not reachable through `scripts/develop.sh`: development builds bypass the // experiment check (`RequireExperimentWithDevBypass`), so a fake is the only // way to exercise this short of a real release binary. t.Run("ExperimentDisabled", func(t *testing.T) { f := newFakeCoderd(t) - f.SetExperimentOff() + // Synthetic wording on purpose: the assertion is that coderd's message + // reaches the user, not that the provider recognises any particular text. + const serverMessage = "" + f.SetForbiddenWithMessage(serverMessage) resource.Test(t, resource.TestCase{ IsUnitTest: true, @@ -682,8 +642,10 @@ resource "coderd_oauth2_provider_settings" "test" { ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { - Config: oauth2SettingsConfig(f.URL, true), - ExpectError: regexp.MustCompile(`(?s)OAuth2 Experiment Not Enabled.*CODER_EXPERIMENTS=oauth2`), + Config: oauth2SettingsConfig(f.URL, true), + // The server's own explanation is surfaced verbatim; the + // provider adds only which operation failed. + ExpectError: regexp.MustCompile(`(?s)unable to create OAuth2 provider settings.*` + regexp.QuoteMeta(serverMessage)), }, }, }) From 7ce3ea779baa73deb434f7c8f28eb1c8968e0103 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Wed, 29 Jul 2026 13:04:39 -0700 Subject: [PATCH 8/9] fix(coderd_oauth2_provider_settings): correct the minimum Coder version to 2.37.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `oauth2ProviderSettingsMinVersion` claimed 2.35.0, which was wrong, and the constant is named in the 404 diagnostic — so an admin on a 2.35.x release was being told to upgrade to a version that would not have fixed their 404. The endpoint arrived with coder/coder#27316, merged 2026-07-28 as `fbac6024`. By then the 2.34, 2.35 and 2.36 release branches were all already cut, and none contains the commit: git merge-base --is-ancestor fbac6024 origin/release/2.34 -> no git merge-base --is-ancestor fbac6024 origin/release/2.35 -> no git merge-base --is-ancestor fbac6024 origin/release/2.36 -> no git tag --contains fbac6024 -> (empty) release/2.36 carries commits dated after the merge, so it was branched beforehand and has taken cherry-picks since — but not this one, and a `feat!` breaking change would not be backported into a cut release branch. That makes 2.37 the first line that can carry the endpoint. Still unreleased: v2.35.3 (2026-07-27) is the newest 2.35.x, and the commit is in no tag. The comment records that this should be re-checked once 2.37.0 ships, in case the release lands under a different number. Addresses the "re-verify 2.35.0" half of the blocking review. The unrelated v2.35.0 references in ai_provider_resource.go are for Bedrock STS assume-role support and are untouched. --- docs/resources/oauth2_provider_settings.md | 4 ++-- .../provider/oauth2_provider_settings_resource.go | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/resources/oauth2_provider_settings.md b/docs/resources/oauth2_provider_settings.md index 94bf35c..6612d4d 100644 --- a/docs/resources/oauth2_provider_settings.md +++ b/docs/resources/oauth2_provider_settings.md @@ -23,7 +23,7 @@ description: |- Dynamic Client Registration alone. Because the setting is a deployment-wide singleton, only one Terraform configuration should declare this resource. ~> Warning - This resource is only compatible with Coder version 2.35.0 https://github.com/coder/coder/releases/tag/v2.35.0 and later. + This resource is only compatible with Coder version 2.37.0 https://github.com/coder/coder/releases/tag/v2.37.0 and later. ~> Warning The deployment must have the oauth2 experiment enabled (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2). It is off by default, and --experiments='*' does not enable it, so it must be named explicitly. Without it, /api/v2/oauth2-provider/settings returns 403 and this resource cannot manage the setting. Development builds of Coder bypass the check, so a -devel deployment works either way — which makes this easy to miss until you apply against a release. --- @@ -55,7 +55,7 @@ Dynamic Client Registration alone. Because the setting is a deployment-wide singleton, only one Terraform configuration should declare this resource. ~> **Warning** -This resource is only compatible with Coder version [2.35.0](https://github.com/coder/coder/releases/tag/v2.35.0) and later. +This resource is only compatible with Coder version [2.37.0](https://github.com/coder/coder/releases/tag/v2.37.0) and later. ~> **Warning** The deployment must have the `oauth2` experiment enabled (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`). It is **off by default**, and `--experiments='*'` does **not** enable it, so it must be named explicitly. Without it, `/api/v2/oauth2-provider/settings` returns `403` and this resource cannot manage the setting. Development builds of Coder bypass the check, so a `-devel` deployment works either way — which makes this easy to miss until you apply against a release. diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index 1bc2881..d15585b 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -24,7 +24,20 @@ var _ resource.ResourceWithModifyPlan = &OAuth2ProviderSettingsResource{} // `/api/v2/oauth2-provider/settings`. It is named in the error surfaced when // that endpoint 404s, so an admin pointed at an older deployment gets an // actionable message instead of a bare "not found". -const oauth2ProviderSettingsMinVersion = "2.35.0" +// +// 2.37.0 rather than the 2.35.0 this originally claimed, which was wrong. The +// endpoint arrived with coder/coder#27316, merged 2026-07-28 as `fbac6024`, and +// at that point the 2.34, 2.35 and 2.36 release branches were all already cut — +// none contains the commit, so no 2.35.x or 2.36.x release can serve the +// endpoint. Telling an admin to upgrade to 2.35.0 would not have fixed their +// 404. The upstream change is a `feat!`, so it will not be backported into a cut +// release branch, making 2.37 the first line that can carry it. +// +// Still unreleased at the time of writing: v2.35.3 (2026-07-27) remains the +// newest 2.35.x, and `git tag --contains fbac6024` is empty. Worth re-checking +// once 2.37.0 actually ships, in case the release lands under a different +// number. +const oauth2ProviderSettingsMinVersion = "2.37.0" // oauth2ProviderSettingsExperiment is the coderd experiment gating the whole // `/api/v2/oauth2-provider` route, settings included. While it is unset, route From 51758a3e8a7deec216244ca9b7888e53ae4b4e3b Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Thu, 30 Jul 2026 09:55:20 -0700 Subject: [PATCH 9/9] docs(coderd_oauth2_provider_settings): tighten schema and example wording Trims duplicated warnings in the schema description and example.tf/import.sh to terser wording per review feedback on #399. --- docs/resources/oauth2_provider_settings.md | 62 ++++--------------- .../coderd_oauth2_provider_settings/import.sh | 10 +-- .../resource.tf | 16 +---- .../oauth2_provider_settings_resource.go | 18 ++---- 4 files changed, 20 insertions(+), 86 deletions(-) diff --git a/docs/resources/oauth2_provider_settings.md b/docs/resources/oauth2_provider_settings.md index 6612d4d..da59747 100644 --- a/docs/resources/oauth2_provider_settings.md +++ b/docs/resources/oauth2_provider_settings.md @@ -4,79 +4,47 @@ page_title: "coderd_oauth2_provider_settings Resource - terraform-provider-coder subcategory: "" description: |- Deployment-wide OAuth2 provider settings. - This setting is a deployment-wide singleton, so this resource can only be - declared once. Declaring it more than once is not an error: whichever block - applies last silently wins. + This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. ~> Warning - When adopting a deployment where this setting has already been configured out - of band (via coder oauth2-provider dcr enable or the deployment settings UI), run - terraform import before your first terraform apply. There is no prior state - for Terraform to diff against on a first apply, so the live value is overwritten - with your configured one without appearing as a change in the plan. A plan-time - warning is raised if this would disable Dynamic Client Registration where it is - currently enabled, but a warning does not block the apply. + If DCR was configured out of band, terraform import this resource before the first apply. Otherwise Terraform overwrites the live value without a diff; disabling an enabled setting emits a non-blocking warning. ~> Warning terraform destroy resets dynamic_client_registration_enabled to false, the deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. - -> Managing this setting is entirely optional: omit the resource to leave - Dynamic Client Registration alone. Because the setting is a deployment-wide - singleton, only one Terraform configuration should declare this resource. + -> Omit this resource to leave DCR unmanaged. Only one Terraform configuration should own it. ~> Warning This resource is only compatible with Coder version 2.37.0 https://github.com/coder/coder/releases/tag/v2.37.0 and later. ~> Warning - The deployment must have the oauth2 experiment enabled (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2). It is off by default, and --experiments='*' does not enable it, so it must be named explicitly. Without it, /api/v2/oauth2-provider/settings returns 403 and this resource cannot manage the setting. Development builds of Coder bypass the check, so a -devel deployment works either way — which makes this easy to miss until you apply against a release. + Requires the oauth2 experiment (CODER_EXPERIMENTS=oauth2 or --experiments=oauth2); * does not enable it. Without it, /api/v2/oauth2-provider/settings returns 403 (development builds bypass this check). --- # coderd_oauth2_provider_settings (Resource) Deployment-wide OAuth2 provider settings. -This setting is a deployment-wide singleton, so this resource can only be -declared once. Declaring it more than once is not an error: whichever block -applies last silently wins. +This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. ~> **Warning** -When adopting a deployment where this setting has already been configured out -of band (via `coder oauth2-provider dcr enable` or the deployment settings UI), run -`terraform import` **before** your first `terraform apply`. There is no prior state -for Terraform to diff against on a first apply, so the live value is overwritten -with your configured one without appearing as a change in the plan. A plan-time -warning is raised if this would disable Dynamic Client Registration where it is -currently enabled, but a warning does not block the apply. +If DCR was configured out of band, `terraform import` this resource before the first apply. Otherwise Terraform overwrites the live value without a diff; disabling an enabled setting emits a non-blocking warning. ~> **Warning** `terraform destroy` resets `dynamic_client_registration_enabled` to `false`, the deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. --> Managing this setting is entirely optional: omit the resource to leave -Dynamic Client Registration alone. Because the setting is a deployment-wide -singleton, only one Terraform configuration should declare this resource. +-> Omit this resource to leave DCR unmanaged. Only one Terraform configuration should own it. ~> **Warning** This resource is only compatible with Coder version [2.37.0](https://github.com/coder/coder/releases/tag/v2.37.0) and later. ~> **Warning** -The deployment must have the `oauth2` experiment enabled (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`). It is **off by default**, and `--experiments='*'` does **not** enable it, so it must be named explicitly. Without it, `/api/v2/oauth2-provider/settings` returns `403` and this resource cannot manage the setting. Development builds of Coder bypass the check, so a `-devel` deployment works either way — which makes this easy to miss until you apply against a release. +Requires the `oauth2` experiment (`CODER_EXPERIMENTS=oauth2` or `--experiments=oauth2`); `*` does not enable it. Without it, `/api/v2/oauth2-provider/settings` returns `403` (development builds bypass this check). ## Example Usage ```terraform -// Prerequisite: the deployment must have the `oauth2` experiment enabled -// (CODER_EXPERIMENTS=oauth2, or --experiments=oauth2). It is off by default and -// is NOT included in `--experiments='*'`, so it has to be named explicitly. -// Without it the API returns 403 and this resource cannot manage the setting. -// (Development builds of Coder bypass the check, so a `-devel` deployment works -// either way -- which makes this easy to miss until you hit a real release.) -// -// Important note: this setting is a deployment-wide singleton, so you can only -// have one resource of this type! Declaring it twice is not an error, the -// blocks will silently overwrite each other on every apply. -// -// If the deployment already has this setting configured (via the CLI or the -// deployment settings UI), run `terraform import` before your first apply, or -// this resource will overwrite the live value without showing a diff. +// Requires the `oauth2` experiment. +// Import first if DCR is already managed out of band. resource "coderd_oauth2_provider_settings" "dcr" { dynamic_client_registration_enabled = true @@ -112,14 +80,8 @@ Import is supported using the following syntax: The [`terraform import` command](https://developer.hashicorp.com/terraform/cli/commands/import) can be used, for example: ```shell -# This setting is a deployment-wide singleton with no identifying attribute, so -# the import ID is an unused placeholder. Any string works; a fixed one keeps -# the command copy-pasteable. -# -# Import before your first apply when adopting a deployment that already has -# this setting configured: state is then populated from the live value, and the -# following `terraform plan` shows an honest diff instead of silently -# overwriting it. +# The singleton has no ID; any placeholder works. +# Import before the first apply to adopt the live value. $ terraform import coderd_oauth2_provider_settings.dcr oauth2_provider_settings ``` Alternatively, in Terraform v1.5.0 and later, an [`import` block](https://developer.hashicorp.com/terraform/language/import) can be used: diff --git a/examples/resources/coderd_oauth2_provider_settings/import.sh b/examples/resources/coderd_oauth2_provider_settings/import.sh index 23f00da..ad1eea0 100644 --- a/examples/resources/coderd_oauth2_provider_settings/import.sh +++ b/examples/resources/coderd_oauth2_provider_settings/import.sh @@ -1,11 +1,5 @@ -# This setting is a deployment-wide singleton with no identifying attribute, so -# the import ID is an unused placeholder. Any string works; a fixed one keeps -# the command copy-pasteable. -# -# Import before your first apply when adopting a deployment that already has -# this setting configured: state is then populated from the live value, and the -# following `terraform plan` shows an honest diff instead of silently -# overwriting it. +# The singleton has no ID; any placeholder works. +# Import before the first apply to adopt the live value. $ terraform import coderd_oauth2_provider_settings.dcr oauth2_provider_settings ``` Alternatively, in Terraform v1.5.0 and later, an [`import` block](https://developer.hashicorp.com/terraform/language/import) can be used: diff --git a/examples/resources/coderd_oauth2_provider_settings/resource.tf b/examples/resources/coderd_oauth2_provider_settings/resource.tf index cf4b863..147f5b8 100644 --- a/examples/resources/coderd_oauth2_provider_settings/resource.tf +++ b/examples/resources/coderd_oauth2_provider_settings/resource.tf @@ -1,17 +1,5 @@ -// Prerequisite: the deployment must have the `oauth2` experiment enabled -// (CODER_EXPERIMENTS=oauth2, or --experiments=oauth2). It is off by default and -// is NOT included in `--experiments='*'`, so it has to be named explicitly. -// Without it the API returns 403 and this resource cannot manage the setting. -// (Development builds of Coder bypass the check, so a `-devel` deployment works -// either way -- which makes this easy to miss until you hit a real release.) -// -// Important note: this setting is a deployment-wide singleton, so you can only -// have one resource of this type! Declaring it twice is not an error, the -// blocks will silently overwrite each other on every apply. -// -// If the deployment already has this setting configured (via the CLI or the -// deployment settings UI), run `terraform import` before your first apply, or -// this resource will overwrite the live value without showing a diff. +// Requires the `oauth2` experiment. +// Import first if DCR is already managed out of band. resource "coderd_oauth2_provider_settings" "dcr" { dynamic_client_registration_enabled = true diff --git a/internal/provider/oauth2_provider_settings_resource.go b/internal/provider/oauth2_provider_settings_resource.go index d15585b..6856d81 100644 --- a/internal/provider/oauth2_provider_settings_resource.go +++ b/internal/provider/oauth2_provider_settings_resource.go @@ -77,33 +77,23 @@ func (r *OAuth2ProviderSettingsResource) Schema(ctx context.Context, req resourc resp.Schema = schema.Schema{ MarkdownDescription: `Deployment-wide OAuth2 provider settings. -This setting is a deployment-wide singleton, so this resource can only be -declared once. Declaring it more than once is not an error: whichever block -applies last silently wins. +This is a deployment-wide singleton. Declare it once; duplicate resources silently overwrite each other. ~> **Warning** -When adopting a deployment where this setting has already been configured out -of band (via ` + "`coder oauth2-provider dcr enable`" + ` or the deployment settings UI), run -` + "`terraform import`" + ` **before** your first ` + "`terraform apply`" + `. There is no prior state -for Terraform to diff against on a first apply, so the live value is overwritten -with your configured one without appearing as a change in the plan. A plan-time -warning is raised if this would disable Dynamic Client Registration where it is -currently enabled, but a warning does not block the apply. +If DCR was configured out of band, ` + "`terraform import`" + ` this resource before the first apply. Otherwise Terraform overwrites the live value without a diff; disabling an enabled setting emits a non-blocking warning. ~> **Warning** ` + "`terraform destroy`" + ` resets ` + "`dynamic_client_registration_enabled`" + ` to ` + "`false`" + `, the deployment default. The API has no delete operation for this setting, so the value cannot be returned to a "never configured" state. --> Managing this setting is entirely optional: omit the resource to leave -Dynamic Client Registration alone. Because the setting is a deployment-wide -singleton, only one Terraform configuration should declare this resource. +-> Omit this resource to leave DCR unmanaged. Only one Terraform configuration should own it. ~> **Warning** This resource is only compatible with Coder version [` + oauth2ProviderSettingsMinVersion + `](https://github.com/coder/coder/releases/tag/v` + oauth2ProviderSettingsMinVersion + `) and later. ~> **Warning** -The deployment must have the ` + "`" + oauth2ProviderSettingsExperiment + "`" + ` experiment enabled (` + "`CODER_EXPERIMENTS=" + oauth2ProviderSettingsExperiment + "`" + ` or ` + "`--experiments=" + oauth2ProviderSettingsExperiment + "`" + `). It is **off by default**, and ` + "`--experiments='*'`" + ` does **not** enable it, so it must be named explicitly. Without it, ` + "`/api/v2/oauth2-provider/settings`" + ` returns ` + "`403`" + ` and this resource cannot manage the setting. Development builds of Coder bypass the check, so a ` + "`-devel`" + ` deployment works either way — which makes this easy to miss until you apply against a release. +Requires the ` + "`" + oauth2ProviderSettingsExperiment + "`" + ` experiment (` + "`CODER_EXPERIMENTS=" + oauth2ProviderSettingsExperiment + "`" + ` or ` + "`--experiments=" + oauth2ProviderSettingsExperiment + "`" + `); ` + "`*`" + ` does not enable it. Without it, ` + "`/api/v2/oauth2-provider/settings`" + ` returns ` + "`403`" + ` (development builds bypass this check). `, Attributes: map[string]schema.Attribute{ "dynamic_client_registration_enabled": schema.BoolAttribute{