Skip to content

CNF-23374: Migrate away from deprecated ioutil #202

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
sebrandon1 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CNF-23374: Migrate away from deprecated ioutil #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/authenticator/password/basicauthpassword/basicauthpassword.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"strings"

Expand Down Expand Up @@ -103,7 +103,7 @@ func (a *Authenticator) AuthenticatePassword(ctx context.Context, username, pass
return nil, false, nil
}

body, err := ioutil.ReadAll(resp.Body)
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, false, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/authenticator/password/keystonepassword/keystonepassword_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"io"
"net/http"
"testing"

Expand Down Expand Up @@ -59,7 +59,7 @@ func TestKeystoneLogin(t *testing.T) {
}
}
var x AuthRequest
body, _ := ioutil.ReadAll(r.Body)
body, _ := io.ReadAll(r.Body)
th.AssertNoErr(t, json.Unmarshal(body, &x))
domainName := x.Auth.Identity.Password.User.Domain.Name
Comment on lines 61 to 64

@coderabbitai coderabbitai Bot Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle request-body read errors in the Keystone test handler.

The handler ignores errors from io.ReadAll(r.Body) (body, _ := ...). If that fails, the subsequent json.Unmarshal assertion won’t clearly indicate the underlying read problem.

💡 Suggested fix 
-		var x AuthRequest
-		body, _ := io.ReadAll(r.Body)
-		th.AssertNoErr(t, json.Unmarshal(body, &x))
+		var x AuthRequest
+		body, err := io.ReadAll(r.Body)
+		th.AssertNoErr(t, err)
+		th.AssertNoErr(t, json.Unmarshal(body, &x))
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/authenticator/password/keystonepassword/keystonepassword_test.go` around
lines 61 - 64, The test HTTP handler currently ignores errors from
io.ReadAll(r.Body) causing json.Unmarshal to mask read failures; change the
handler to capture and assert the read error before unmarshalling (e.g., check
the error returned by io.ReadAll and call th.AssertNoErr or t.Fatalf with that
error and return), so the code around the AuthRequest parsing (variables x
AuthRequest, the io.ReadAll call and subsequent json.Unmarshal) fails fast with
a clear error if the body read fails.

@liouk liouk May 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, this seems like a valid addition/fix here.

@coderabbitai coderabbitai Bot May 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@liouk, thanks for the confirmation! Here is the suggested fix for the PR author to apply:

 		var x AuthRequest
-		body, _ := io.ReadAll(r.Body)
-		th.AssertNoErr(t, json.Unmarshal(body, &x))
+		body, err := io.ReadAll(r.Body)
+		th.AssertNoErr(t, err)
+		th.AssertNoErr(t, json.Unmarshal(body, &x))

This ensures a body-read failure surfaces immediately with a clear error, rather than being silently swallowed and potentially causing a confusing json.Unmarshal failure downstream.

userName := x.Auth.Identity.Password.User.Name
Expand Down
5 changes: 2 additions & 3 deletions pkg/config/stringsource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"crypto/x509"
"encoding/pem"
"fmt"
"io/ioutil"
"os"

"k8s.io/apimachinery/pkg/util/sets"
Expand All @@ -31,7 +30,7 @@ func ResolveStringValue(s configv1.StringSource) (string, error) {
case len(s.Env) > 0:
value = os.Getenv(s.Env)
case len(s.File) > 0:
data, err := ioutil.ReadFile(s.File)
data, err := os.ReadFile(s.File)
if err != nil {
return "", err
}
Expand All @@ -45,7 +44,7 @@ func ResolveStringValue(s configv1.StringSource) (string, error) {
return value, nil
}

keyData, err := ioutil.ReadFile(s.KeyFile)
keyData, err := os.ReadFile(s.KeyFile)
if err != nil {
return "", err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/oauth/external/gitlab/gitlab_oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"net/url"

Expand Down Expand Up @@ -95,7 +95,7 @@ func (p *provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdenti
}
defer res.Body.Close()

body, err := ioutil.ReadAll(res.Body)
body, err := io.ReadAll(res.Body)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/oauth/external/openid/openid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -273,7 +273,7 @@ func fetchUserInfo(url, accessToken string, transport http.RoundTripper) (map[st

// The UserInfo Claims MUST be returned as the members of a JSON object
// http://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
data, err := ioutil.ReadAll(resp.Body)
data, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/oauthserver/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (
"crypto/x509"
"errors"
"fmt"
"io/ioutil"
"net/http"
"net/url"
"os"
"path"
"strings"

Expand Down Expand Up @@ -685,7 +685,7 @@ func (c *OAuthServerConfig) getAuthenticationRequestHandler() (authenticator.Req

// Wrap with an x509 verifier
if len(provider.ClientCA) > 0 {
caData, err := ioutil.ReadFile(provider.ClientCA)
caData, err := os.ReadFile(provider.ClientCA)
if err != nil {
return nil, fmt.Errorf("Error reading %s: %v", provider.ClientCA, err)
}
Expand Down
13 changes: 6 additions & 7 deletions pkg/oauthserver/oauth_apiserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package oauthserver
import (
"context"
"fmt"
"io/ioutil"
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -35,13 +34,13 @@ func TestGetMissingSessionSecretsFile(t *testing.T) {
}

func TestGetInvalidSessionSecretsFile(t *testing.T) {
tmpfile, err := ioutil.TempFile("", "invalid.yaml")
tmpfile, err := os.CreateTemp("", "invalid.yaml")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
defer os.Remove(tmpfile.Name())

if err := ioutil.WriteFile(tmpfile.Name(), []byte("invalid content"), os.FileMode(0600)); err != nil {
if err := os.WriteFile(tmpfile.Name(), []byte("invalid content"), os.FileMode(0600)); err != nil {
t.Fatal(err)
}
Comment on lines +43 to 45

@coderabbitai coderabbitai Bot Apr 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use 0o600 instead of 0600 for file mode literals (style/clarity).

These tests use os.FileMode(0600) when writing temp files. Consider switching to os.FileMode(0o600) (Go’s modern octal literal form) for readability/consistency.

Also applies to: 61-63, 90-92

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/oauthserver/oauth_apiserver_test.go` around lines 36 - 38, Update the
octal file mode literals in oauth_apiserver_test.go to use Go's modern form:
replace occurrences of os.FileMode(0600) with os.FileMode(0o600) wherever
os.WriteFile is called (e.g., the os.WriteFile(tmpfile.Name(), ... ,
os.FileMode(0600)) invocations around the tmpfile writes at the three places
noted). Keep the same type and semantics, only change the numeric literal to
0o600 for readability and consistency.


Expand All @@ -52,7 +51,7 @@ func TestGetInvalidSessionSecretsFile(t *testing.T) {
}

func TestGetEmptySessionSecretsFile(t *testing.T) {
tmpfile, err := ioutil.TempFile("", "empty.yaml")
tmpfile, err := os.CreateTemp("", "empty.yaml")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -66,7 +65,7 @@ func TestGetEmptySessionSecretsFile(t *testing.T) {
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if err := ioutil.WriteFile(tmpfile.Name(), []byte(yaml), os.FileMode(0600)); err != nil {
if err := os.WriteFile(tmpfile.Name(), []byte(yaml), os.FileMode(0600)); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -168,7 +167,7 @@ func TestConfigureTransport_ProxyCAFileNotFound(t *testing.T) {
}

func TestGetValidSessionSecretsFile(t *testing.T) {
tmpfile, err := ioutil.TempFile("", "valid.yaml")
tmpfile, err := os.CreateTemp("", "valid.yaml")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -186,7 +185,7 @@ func TestGetValidSessionSecretsFile(t *testing.T) {
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if err := ioutil.WriteFile(tmpfile.Name(), []byte(yaml), os.FileMode(0600)); err != nil {
if err := os.WriteFile(tmpfile.Name(), []byte(yaml), os.FileMode(0600)); err != nil {
t.Fatal(err)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/server/login/login_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package login
import (
"context"
"errors"
"io/ioutil"
"io"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -246,7 +246,7 @@ func TestLogin(t *testing.T) {
}

if len(testCase.ExpectContains) > 0 {
data, _ := ioutil.ReadAll(resp.Body)
data, _ := io.ReadAll(resp.Body)
body := string(data)
for i := range testCase.ExpectContains {
if !strings.Contains(body, testCase.ExpectContains[i]) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/selectprovider/selectprovider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package selectprovider

import (
"io/ioutil"
"io"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestSelectAuthentication(t *testing.T) {
}

if len(testCase.ExpectContains) > 0 {
data, _ := ioutil.ReadAll(resp.Body)
data, _ := io.ReadAll(resp.Body)
body := string(data)
for i := range testCase.ExpectContains {
if !strings.Contains(body, testCase.ExpectContains[i]) {
Expand Down