Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 0 deletions pkg/certificate/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
return w.Save()
}

identityProvisioningRequest := func(forceInit bool) (err error, identity *InstanceIdentity, keyPEM []byte) {

Check failure on line 159 in pkg/certificate/service.go

View workflow job for this annotation

GitHub Actions / Run unit test / go-test

Function `New$2->ApplyX509CertToSecret->ApplyIdentitySecret` should pass the context parameter (contextcheck)
log.Infof("Mapped Athenz domain[%s], service[%s]", handler.Domain(), handler.Service())

identity, keyPEM, err = handler.GetX509Cert(forceInit)
Expand Down Expand Up @@ -208,7 +208,11 @@
return nil, roleCerts, roleKeyPEM
}

run := func() error {

Check failure on line 211 in pkg/certificate/service.go

View workflow job for this annotation

GitHub Actions / Run unit test / go-test

Function `New$4->GetX509CertFromSecret->GetIdentitySecret` should pass the context parameter (contextcheck)
if err := idCfg.ValidateCertFilePath(); err != nil {
return err
}

if idCfg.ServiceCert.CopperArgos.Use {
log.Infof("Attempting to request x509 certificate to identity provider[%s]...", idCfg.ServiceCert.CopperArgos.Provider)

Expand Down
69 changes: 69 additions & 0 deletions pkg/config/validate-file-path.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2023 LY Corporation
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package config

import (
"fmt"
"os"
"path/filepath"

"golang.org/x/sys/unix"
)

func validFilePath(file_path string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Write a comment please.

  • What it does
  • Why we have this

dir_path := filepath.Dir(file_path)
var target_path string
_, file_err := os.Stat(file_path)
_, dir_err := os.Stat(dir_path)

if file_err == nil {
// validate file path
target_path = file_path
} else if os.IsNotExist(file_err) && dir_err == nil {
// validate dir path
target_path = dir_path
} else {
return fmt.Errorf("file path not exist: %w, %w", file_err, dir_err)
Copy link

Copilot AI Aug 25, 2025

Choose a reason for hiding this comment

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

The error message has grammatical issues. Change 'file path not exist' to 'file path does not exist'.

Suggested change
return fmt.Errorf("file path not exist: %w, %w", file_err, dir_err)
return fmt.Errorf("file path does not exist: %w, %w", file_err, dir_err)

Copilot uses AI. Check for mistakes.
}
Comment on lines +26 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

was wondering if you can write a separate function for this, instead of including within the validFilePath()

func getPath(path string) error, string {
  _, err:= os.Stat(filePath)
  if err == nil {
     return nil, filePath
  }

  if !os.IsNotExist(err) {
     return  "", fmt.Errorf("error accessing path '%s': %w", filePath, err)
  }
 
  dirPath := filepath.Dir(filePath)
   if _, dirErr := osStat(dirPath); dirErr != nil { return "", fmt.Errorf("todo error"); }
  return dirPath, nil
}

Then just do

	if err := unix.Access(getPath(path), unix.W_OK); err != nil {
		return fmt.Errorf("file permission error: %w", err)
	}

Copy link
Contributor

Choose a reason for hiding this comment

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

Did not write ocmments or good function name so please do so.


err := unix.Access(target_path, unix.W_OK)
if err != nil {
return fmt.Errorf("file permission error: %w", err)
}
Comment on lines +41 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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


return nil
}

// Verify that the certificate file paths are in writable locations
func (idCfg *IdentityConfig) ValidateCertFilePath() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a background why we do this
(Can we have more comments for "why" this function exists)

// When idCfg.ServiceCert.LocalCert.Use is true, skip file writing and return early
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a why please thanks. Why do we skip for LocalCert Mode?

if idCfg.ServiceCert.LocalCert.Use {
return nil
}

for _, certFile := range idCfg.ServiceCert.CopperArgos.Cert.Paths {
err := validFilePath(certFile)
if err != nil {
return err
}
Comment on lines +57 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

}
for _, keyFile := range idCfg.ServiceCert.CopperArgos.Key.Paths {
err := validFilePath(keyFile)
if err != nil {
return err
}
Comment on lines +63 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Nits but:

Suggested change
err := validFilePath(keyFile)
if err != nil {
return err
}
if err := validFilePath(keyFile); err != nil {
return err
}

}
return validFilePath(idCfg.CaCertFile)
}
Loading