diff --git a/go.mod b/go.mod index 3936d049ab..5181222bdb 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.3.0 github.com/containers/buildah v1.31.1-0.20230722114901-5ece066f82c6 - github.com/containers/common v0.55.1-0.20230816154734-519ed7fea9bd + github.com/containers/common v0.55.1-0.20230824140149-b27c2ba2b7e1 github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.26.1-0.20230807184415-3fb422379cfa github.com/containers/libhvee v0.4.0 @@ -132,7 +132,7 @@ require ( github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-retryablehttp v0.7.4 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/jinzhu/copier v0.3.5 // indirect + github.com/jinzhu/copier v0.4.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/klauspost/compress v1.16.7 // indirect github.com/klauspost/cpuid/v2 v2.2.4 // indirect diff --git a/go.sum b/go.sum index 680f5895c1..68eb2eaee0 100644 --- a/go.sum +++ b/go.sum @@ -246,8 +246,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0= github.com/containers/buildah v1.31.1-0.20230722114901-5ece066f82c6 h1:K/S8SFQsnnNTF0Ws58SrBD9L0EuClzAG8Zp08d7+6AA= github.com/containers/buildah v1.31.1-0.20230722114901-5ece066f82c6/go.mod h1:0sptTFBBtSznLqoTh80DfvMOCNbdRsNRgVOKhBhrupA= -github.com/containers/common v0.55.1-0.20230816154734-519ed7fea9bd h1:fdpl099M/XfhB+yYACXFcW7Dsaz+1oHs57hF78SR9wI= -github.com/containers/common v0.55.1-0.20230816154734-519ed7fea9bd/go.mod h1:wtIdVQKHf4U+UfIz9B1htNZqqEeMNysQOevHNiNrru0= +github.com/containers/common v0.55.1-0.20230824140149-b27c2ba2b7e1 h1:MfMnYctOK+QlYQudW88avvOzU/kBVWdqs+w40inJeKM= +github.com/containers/common v0.55.1-0.20230824140149-b27c2ba2b7e1/go.mod h1:1ZeaFdUqfLR9HE45ZacZwO3m4y5UOWP/DY9oZlTOYlI= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.26.1-0.20230807184415-3fb422379cfa h1:wDfVQtc6ik2MvsUmu/YRSyBAE5YUxdjcEDtuT1q2KDo= @@ -637,8 +637,8 @@ github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANyt github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6tORTn+6F6j+Jc8TOr5osrynvN6ivFWZ2GA= -github.com/jinzhu/copier v0.3.5 h1:GlvfUwHk62RokgqVNvYsku0TATCF7bAHVwEXoBh3iJg= -github.com/jinzhu/copier v0.3.5/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg= +github.com/jinzhu/copier v0.4.0 h1:w3ciUoD19shMCRargcpm0cm91ytaBhDvuRpz1ODO/U8= +github.com/jinzhu/copier v0.4.0/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg= github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20160803190731-bd40a432e4c7/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmhodges/clock v0.0.0-20160418191101-880ee4c33548 h1:dYTbLf4m0a5u0KLmPfB6mgxbcV7588bOCx79hxa5Sr4= diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 844863a428..43cc30f07f 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -167,7 +167,8 @@ func newServer(runtime *libpod.Runtime, listener net.Listener, opts entities.Ser // setupSystemd notifies systemd API service is ready // If the NOTIFY_SOCKET is set, communicate the PID and readiness, and unset INVOCATION_ID -// so conmon and containers are in the correct cgroup. +// so conmon and containers are in the correct cgroup. Also unset NOTIFY_SOCKET +// to avoid any further usage of the socket. func (s *APIServer) setupSystemd() { if _, found := os.LookupEnv("NOTIFY_SOCKET"); !found { return @@ -176,13 +177,16 @@ func (s *APIServer) setupSystemd() { payload := fmt.Sprintf("MAINPID=%d\n", os.Getpid()) payload += daemon.SdNotifyReady if sent, err := daemon.SdNotify(true, payload); err != nil { - logrus.Error("API service failed to notify systemd of Conmon PID: " + err.Error()) + logrus.Errorf("API service failed to notify systemd of Conmon PID: %v", err) } else if !sent { logrus.Warn("API service unable to successfully send SDNotify") } if err := os.Unsetenv("INVOCATION_ID"); err != nil { - logrus.Error("API service failed unsetting INVOCATION_ID: " + err.Error()) + logrus.Errorf("API service failed unsetting INVOCATION_ID: %v", err) + } + if err := os.Unsetenv("NOTIFY_SOCKET"); err != nil { + logrus.Errorf("API service failed unsetting NOTIFY_SOCKET: %v", err) } } diff --git a/test/apiv2/60-auth.at b/test/apiv2/60-auth.at index 433da11be6..21b1478140 100644 --- a/test/apiv2/60-auth.at +++ b/test/apiv2/60-auth.at @@ -6,7 +6,7 @@ start_registry htpasswd # Test unreachable -t POST /v1.40/auth username=$REGISTRY_USERNAME password=WrOnGPassWord serveraddress=does.not.exist.io:1234/ \ +t POST /v1.40/auth username=$REGISTRY_USERNAME password=WrOnGPassWord serveraddress=doesnotexists.podman.io:1234/ \ 500 \ .message~'.*no such host.*' diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index ce45c7f851..165d3325d7 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -4,6 +4,8 @@ # load helpers +load helpers.network +load helpers.registry # Shared throughout this module: PID of socat process, and path to its log _SOCAT_PID= @@ -505,4 +507,85 @@ none | false | false | 0 run_podman rmi $(pause_image) } + +@test "podman pull - EXTEND_TIMEOUT_USEC" { + # Make sure that Podman extends the start timeout via DBUS when running + # inside a systemd unit (i.e., with NOTIFY_SOCKET set). Extending the + # timout works by continuously sending EXTEND_TIMEOUT_USEC; Podman does + # this at most 10 times, adding up to ~5min. + + image_on_local_registry=localhost:${PODMAN_LOGIN_REGISTRY_PORT}/name:tag + registry_flags="--tls-verify=false --creds ${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" + start_registry + + export NOTIFY_SOCKET=$PODMAN_TMPDIR/notify.sock + _start_socat + + run_podman push $registry_flags $IMAGE $image_on_local_registry + run_podman pull $registry_flags $image_on_local_registry + is "${lines[1]}" "Pulling image $image_on_local_registry inside systemd: setting pull timeout to 5m0s" "NOTIFY_SOCKET is passed to container" + + run cat $_SOCAT_LOG + # The 'echo's help us debug failed runs + echo "socat log:" + echo "$output" + is "$output" "EXTEND_TIMEOUT_USEC=30000000" + + run_podman rmi $image_on_local_registry + _stop_socat +} + +@test "podman system service" { + # This test makes sure that podman-system-service uses the NOTIFY_SOCKET + # correctly and that it unsets it after sending the expected MAINPID and + # READY message by making sure no EXTEND_TIMEOUT_USEC is sent on pull. + + # Start a local registry and pre-populate it with an image we'll pull later on. + image_on_local_registry=localhost:${PODMAN_LOGIN_REGISTRY_PORT}/name:tag + registry_flags="--tls-verify=false --creds ${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" + start_registry + run_podman push $registry_flags $IMAGE $image_on_local_registry + + export NOTIFY_SOCKET=$PODMAN_TMPDIR/notify.sock + podman_socket="unix://$PODMAN_TMPDIR/podman.sock" + envfile=$PODMAN_TMPDIR/envfile + _start_socat + + (timeout --foreground -v --kill=10 30 $PODMAN system service -t0 $podman_socket &) + + wait_for_file $_SOCAT_LOG + local timeout=10 + while [[ $timeout -gt 0 ]]; do + run cat $_SOCAT_LOG + # The 'echo's help us debug failed runs + echo "socat log:" + echo "$output" + + if [[ "$output" =~ "READY=1" ]]; then + break + fi + timeout=$((timeout - 1)) + assert $timeout -gt 0 "Timed out waiting for podman-system-service to send expected data over NOTIFY_SOCKET" + sleep 0.5 + done + + assert "$output" =~ "MAINPID=.* +READY=1" "podman-system-service sends expected data over NOTIFY_SOCKET" + mainpid=${lines[0]:8} + + # Now pull remotely and make sure that the service does _not_ extend the + # timeout; the NOTIFY_SOCKET should be unset at that point. + run_podman --url $podman_socket pull $registry_flags $image_on_local_registry + + run cat $_SOCAT_LOG + # The 'echo's help us debug failed runs + echo "socat log:" + echo "$output" + assert "$output" !~ "EXTEND_TIMEOUT_USEC=" + + # Give the system-service 5sec to terminate before killing it. + /bin/kill --timeout 5000 KILL --signal TERM $mainpid + run_podman rmi $image_on_local_registry + _stop_socat +} # vim: filetype=sh diff --git a/vendor/github.com/containers/common/libimage/copier.go b/vendor/github.com/containers/common/libimage/copier.go index b622ef5e33..7a6f1f1b01 100644 --- a/vendor/github.com/containers/common/libimage/copier.go +++ b/vendor/github.com/containers/common/libimage/copier.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "net" "os" "strings" "time" @@ -150,14 +151,19 @@ type CopyOptions struct { // Additional tags when creating or copying a docker-archive. dockerArchiveAdditionalTags []reference.NamedTagged + + // If set it points to a NOTIFY_SOCKET the copier will use to extend + // the systemd timeout while copying. + extendTimeoutSocket string } // copier is an internal helper to conveniently copy images. type copier struct { - imageCopyOptions copy.Options - retryOptions retry.Options - systemContext *types.SystemContext - policyContext *signature.PolicyContext + extendTimeoutSocket string + imageCopyOptions copy.Options + retryOptions retry.Options + systemContext *types.SystemContext + policyContext *signature.PolicyContext sourceLookup LookupReferenceFunc destinationLookup LookupReferenceFunc @@ -208,7 +214,7 @@ func getDockerAuthConfig(name, passwd, creds, idToken string) (*types.DockerAuth // counterparts of the specified system context. Please make sure to call // `(*copier).close()`. func (r *Runtime) newCopier(options *CopyOptions) (*copier, error) { - c := copier{} + c := copier{extendTimeoutSocket: options.extendTimeoutSocket} c.systemContext = r.systemContextCopy() if options.SourceLookupReferenceFunc != nil { @@ -333,6 +339,61 @@ func (c *copier) close() error { func (c *copier) copy(ctx context.Context, source, destination types.ImageReference) ([]byte, error) { logrus.Debugf("Copying source image %s to destination image %s", source.StringWithinTransport(), destination.StringWithinTransport()) + // Avoid running out of time when running inside a systemd unit by + // regularly increasing the timeout. + if c.extendTimeoutSocket != "" { + socketAddr := &net.UnixAddr{ + Name: c.extendTimeoutSocket, + Net: "unixgram", + } + conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr) + if err != nil { + return nil, err + } + defer conn.Close() + + numExtensions := 10 + extension := 30 * time.Second + timerFrequency := 25 * time.Second // Fire the timer at a higher frequency to avoid a race + timer := time.NewTicker(timerFrequency) + socketCtx, cancel := context.WithCancel(ctx) + defer cancel() + defer timer.Stop() + + fmt.Fprintf(c.imageCopyOptions.ReportWriter, "Pulling image %s inside systemd: setting pull timeout to %s\n", source.DockerReference(), time.Duration(numExtensions)*extension) + + // From `man systemd.service(5)`: + // + // "If a service of Type=notify/Type=notify-reload sends "EXTEND_TIMEOUT_USEC=...", this may cause + // the start time to be extended beyond TimeoutStartSec=. The first receipt of this message must + // occur before TimeoutStartSec= is exceeded, and once the start time has extended beyond + // TimeoutStartSec=, the service manager will allow the service to continue to start, provided the + // service repeats "EXTEND_TIMEOUT_USEC=..." within the interval specified until the service startup + // status is finished by "READY=1"." + extendValue := []byte(fmt.Sprintf("EXTEND_TIMEOUT_USEC=%d", extension.Microseconds())) + extendTimeout := func() { + if _, err := conn.Write(extendValue); err != nil { + logrus.Errorf("Increasing EXTEND_TIMEOUT_USEC failed: %v", err) + } + numExtensions-- + } + + extendTimeout() + go func() { + for { + select { + case <-socketCtx.Done(): + return + case <-timer.C: + if numExtensions == 0 { + return + } + extendTimeout() + } + } + }() + } + var err error if c.sourceLookup != nil { diff --git a/vendor/github.com/containers/common/libimage/pull.go b/vendor/github.com/containers/common/libimage/pull.go index 296d003a84..1e8cc42c2d 100644 --- a/vendor/github.com/containers/common/libimage/pull.go +++ b/vendor/github.com/containers/common/libimage/pull.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "os" "runtime" "strings" "time" @@ -592,6 +593,9 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str return nil } + if socketPath, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + options.extendTimeoutSocket = socketPath + } c, err := r.newCopier(&options.CopyOptions) if err != nil { return nil, err diff --git a/vendor/github.com/containers/common/libnetwork/cni/run_freebsd.go b/vendor/github.com/containers/common/libnetwork/cni/run_freebsd.go index c356a864a7..cca00aa83f 100644 --- a/vendor/github.com/containers/common/libnetwork/cni/run_freebsd.go +++ b/vendor/github.com/containers/common/libnetwork/cni/run_freebsd.go @@ -8,6 +8,12 @@ import ( // add the default address. Note: this will also add ::1 as a side // effect. func setupLoopback(namespacePath string) error { - // The jexec wrapper runs the ifconfig command inside the jail. + // Try to run the command using ifconfig's -j flag (supported in 13.3 and later) + if err := exec.Command("ifconfig", "-j", namespacePath, "lo0", "inet", "127.0.0.1").Run(); err == nil { + return nil + } + + // Fall back to using the jexec wrapper to run the ifconfig command + // inside the jail. return exec.Command("jexec", namespacePath, "ifconfig", "lo0", "inet", "127.0.0.1").Run() } diff --git a/vendor/github.com/jinzhu/copier/.gitignore b/vendor/github.com/jinzhu/copier/.gitignore new file mode 100644 index 0000000000..6d742b37a2 --- /dev/null +++ b/vendor/github.com/jinzhu/copier/.gitignore @@ -0,0 +1,2 @@ +.idea/ +ttt/ diff --git a/vendor/github.com/jinzhu/copier/README.md b/vendor/github.com/jinzhu/copier/README.md index ec04b4be0b..079dc5789b 100644 --- a/vendor/github.com/jinzhu/copier/README.md +++ b/vendor/github.com/jinzhu/copier/README.md @@ -30,7 +30,7 @@ type User struct { Name string Role string Age int32 - EmployeCode int64 `copier:"EmployeNum"` // specify field name + EmployeeCode int64 `copier:"EmployeeNum"` // specify field name // Explicitly ignored in the destination struct. Salary int @@ -53,7 +53,7 @@ type Employee struct { Salary int `copier:"-"` DoubleAge int32 - EmployeId int64 `copier:"EmployeNum"` // specify field name + EmployeeId int64 `copier:"EmployeeNum"` // specify field name SuperRole string } @@ -86,7 +86,7 @@ func main() { fmt.Printf("%#v \n", employees) // []Employee{ - // {Name: "Jinzhu", Age: 18, Salary:0, DoubleAge: 36, EmployeId: 0, SuperRole: "Super Admin"} + // {Name: "Jinzhu", Age: 18, Salary:0, DoubleAge: 36, EmployeeId: 0, SuperRole: "Super Admin"} // } // Copy slice to slice @@ -95,8 +95,8 @@ func main() { fmt.Printf("%#v \n", employees) // []Employee{ - // {Name: "Jinzhu", Age: 18, Salary:0, DoubleAge: 36, EmployeId: 0, SuperRole: "Super Admin"}, - // {Name: "jinzhu 2", Age: 30, Salary:0, DoubleAge: 60, EmployeId: 0, SuperRole: "Super Dev"}, + // {Name: "Jinzhu", Age: 18, Salary:0, DoubleAge: 36, EmployeeId: 0, SuperRole: "Super Admin"}, + // {Name: "jinzhu 2", Age: 30, Salary:0, DoubleAge: 60, EmployeeId: 0, SuperRole: "Super Dev"}, // } // Copy map to map diff --git a/vendor/github.com/jinzhu/copier/copier.go b/vendor/github.com/jinzhu/copier/copier.go index 6dc9600c86..43a14f1abc 100644 --- a/vendor/github.com/jinzhu/copier/copier.go +++ b/vendor/github.com/jinzhu/copier/copier.go @@ -3,10 +3,10 @@ package copier import ( "database/sql" "database/sql/driver" - "errors" "fmt" "reflect" "strings" + "sync" "unicode" ) @@ -37,15 +37,35 @@ const ( type Option struct { // setting this value to true will ignore copying zero values of all the fields, including bools, as well as a // struct having all it's fields set to their zero values respectively (see IsZero() in reflect/value.go) - IgnoreEmpty bool - DeepCopy bool - Converters []TypeConverter + IgnoreEmpty bool + CaseSensitive bool + DeepCopy bool + Converters []TypeConverter + // Custom field name mappings to copy values with different names in `fromValue` and `toValue` types. + // Examples can be found in `copier_field_name_mapping_test.go`. + FieldNameMapping []FieldNameMapping +} + +func (opt Option) converters() map[converterPair]TypeConverter { + var converters = map[converterPair]TypeConverter{} + + // save converters into map for faster lookup + for i := range opt.Converters { + pair := converterPair{ + SrcType: reflect.TypeOf(opt.Converters[i].SrcType), + DstType: reflect.TypeOf(opt.Converters[i].DstType), + } + + converters[pair] = opt.Converters[i] + } + + return converters } type TypeConverter struct { SrcType interface{} DstType interface{} - Fn func(src interface{}) (interface{}, error) + Fn func(src interface{}) (dst interface{}, err error) } type converterPair struct { @@ -53,6 +73,27 @@ type converterPair struct { DstType reflect.Type } +func (opt Option) fieldNameMapping() map[converterPair]FieldNameMapping { + var mapping = map[converterPair]FieldNameMapping{} + + for i := range opt.FieldNameMapping { + pair := converterPair{ + SrcType: reflect.TypeOf(opt.FieldNameMapping[i].SrcType), + DstType: reflect.TypeOf(opt.FieldNameMapping[i].DstType), + } + + mapping[pair] = opt.FieldNameMapping[i] + } + + return mapping +} + +type FieldNameMapping struct { + SrcType interface{} + DstType interface{} + Mapping map[string]string +} + // Tag Flags type flags struct { BitFlags map[string]uint8 @@ -82,23 +123,10 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) amount = 1 from = indirect(reflect.ValueOf(fromValue)) to = indirect(reflect.ValueOf(toValue)) - converters map[converterPair]TypeConverter + converters = opt.converters() + mappings = opt.fieldNameMapping() ) - // save convertes into map for faster lookup - for i := range opt.Converters { - if converters == nil { - converters = make(map[converterPair]TypeConverter) - } - - pair := converterPair{ - SrcType: reflect.TypeOf(opt.Converters[i].SrcType), - DstType: reflect.TypeOf(opt.Converters[i].DstType), - } - - converters[pair] = opt.Converters[i] - } - if !to.CanAddr() { return ErrInvalidCopyDestination } @@ -147,7 +175,11 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) for _, k := range from.MapKeys() { toKey := indirect(reflect.New(toType.Key())) - if !set(toKey, k, opt.DeepCopy, converters) { + isSet, err := set(toKey, k, opt.DeepCopy, converters) + if err != nil { + return err + } + if !isSet { return fmt.Errorf("%w map, old key: %v, new key: %v", ErrNotSupported, k.Type(), toType.Key()) } @@ -156,7 +188,11 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) elemType, _ = indirectType(elemType) } toValue := indirect(reflect.New(elemType)) - if !set(toValue, from.MapIndex(k), opt.DeepCopy, converters) { + isSet, err = set(toValue, from.MapIndex(k), opt.DeepCopy, converters) + if err != nil { + return err + } + if !isSet { if err = copier(toValue.Addr().Interface(), from.MapIndex(k).Interface(), opt); err != nil { return err } @@ -174,26 +210,30 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) return } - if from.Kind() == reflect.Slice && to.Kind() == reflect.Slice && fromType.ConvertibleTo(toType) { + if from.Kind() == reflect.Slice && to.Kind() == reflect.Slice { if to.IsNil() { slice := reflect.MakeSlice(reflect.SliceOf(to.Type().Elem()), from.Len(), from.Cap()) to.Set(slice) } - - for i := 0; i < from.Len(); i++ { - if to.Len() < i+1 { - to.Set(reflect.Append(to, reflect.New(to.Type().Elem()).Elem())) - } - - if !set(to.Index(i), from.Index(i), opt.DeepCopy, converters) { - // ignore error while copy slice element - err = copier(to.Index(i).Addr().Interface(), from.Index(i).Interface(), opt) + if fromType.ConvertibleTo(toType) { + for i := 0; i < from.Len(); i++ { + if to.Len() < i+1 { + to.Set(reflect.Append(to, reflect.New(to.Type().Elem()).Elem())) + } + isSet, err := set(to.Index(i), from.Index(i), opt.DeepCopy, converters) if err != nil { - continue + return err + } + if !isSet { + // ignore error while copy slice element + err = copier(to.Index(i).Addr().Interface(), from.Index(i).Interface(), opt) + if err != nil { + continue + } } } + return } - return } if fromType.Kind() != reflect.Struct || toType.Kind() != reflect.Struct { @@ -201,6 +241,13 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) return } + if len(converters) > 0 { + if ok, e := set(to, from, opt.DeepCopy, converters); e == nil && ok { + // converter supported + return + } + } + if from.Kind() == reflect.Slice || to.Kind() == reflect.Slice { isSlice = true if from.Kind() == reflect.Slice { @@ -225,6 +272,27 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) dest = indirect(to) } + if len(converters) > 0 { + if ok, e := set(dest, source, opt.DeepCopy, converters); e == nil && ok { + if isSlice { + // FIXME: maybe should check the other types? + if to.Type().Elem().Kind() == reflect.Ptr { + to.Index(i).Set(dest.Addr()) + } else { + if to.Len() < i+1 { + reflect.Append(to, dest) + } else { + to.Index(i).Set(dest) + } + } + } else { + to.Set(dest) + } + + continue + } + } + destKind := dest.Kind() initDest := false if destKind == reflect.Interface { @@ -248,19 +316,22 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) name := field.Name // Get bit flags for field - fieldFlags, _ := flgs.BitFlags[name] + fieldFlags := flgs.BitFlags[name] // Check if we should ignore copying if (fieldFlags & tagIgnore) != 0 { continue } - srcFieldName, destFieldName := getFieldName(name, flgs) - if fromField := source.FieldByName(srcFieldName); fromField.IsValid() && !shouldIgnore(fromField, opt.IgnoreEmpty) { + fieldNamesMapping := getFieldNamesMapping(mappings, fromType, toType) + + srcFieldName, destFieldName := getFieldName(name, flgs, fieldNamesMapping) + if fromField := fieldByNameOrZeroValue(source, srcFieldName); fromField.IsValid() && !shouldIgnore(fromField, opt.IgnoreEmpty) { // process for nested anonymous field destFieldNotSet := false if f, ok := dest.Type().FieldByName(destFieldName); ok { - for idx := range f.Index { + // only initialize parent embedded struct pointer in the path + for idx := range f.Index[:len(f.Index)-1] { destField := dest.FieldByIndex(f.Index[:idx+1]) if destField.Kind() != reflect.Ptr { @@ -285,10 +356,14 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) break } - toField := dest.FieldByName(destFieldName) + toField := fieldByName(dest, destFieldName, opt.CaseSensitive) if toField.IsValid() { if toField.CanSet() { - if !set(toField, fromField, opt.DeepCopy, converters) { + isSet, err := set(toField, fromField, opt.DeepCopy, converters) + if err != nil { + return err + } + if !isSet { if err := copier(toField.Addr().Interface(), fromField.Interface(), opt); err != nil { return err } @@ -317,7 +392,7 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) // Copy from from method to dest field for _, field := range deepFields(toType) { name := field.Name - srcFieldName, destFieldName := getFieldName(name, flgs) + srcFieldName, destFieldName := getFieldName(name, flgs, getFieldNamesMapping(mappings, fromType, toType)) var fromMethod reflect.Value if source.CanAddr() { @@ -327,7 +402,7 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) } if fromMethod.IsValid() && fromMethod.Type().NumIn() == 0 && fromMethod.Type().NumOut() == 1 && !shouldIgnore(fromMethod, opt.IgnoreEmpty) { - if toField := dest.FieldByName(destFieldName); toField.IsValid() && toField.CanSet() { + if toField := fieldByName(dest, destFieldName, opt.CaseSensitive); toField.IsValid() && toField.CanSet() { values := fromMethod.Call([]reflect.Value{}) if len(values) >= 1 { set(toField, values[0], opt.DeepCopy, converters) @@ -342,7 +417,11 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) if to.Len() < i+1 { to.Set(reflect.Append(to, dest.Addr())) } else { - if !set(to.Index(i), dest.Addr(), opt.DeepCopy, converters) { + isSet, err := set(to.Index(i), dest.Addr(), opt.DeepCopy, converters) + if err != nil { + return err + } + if !isSet { // ignore error while copy slice element err = copier(to.Index(i).Addr().Interface(), dest.Addr().Interface(), opt) if err != nil { @@ -354,7 +433,11 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) if to.Len() < i+1 { to.Set(reflect.Append(to, dest)) } else { - if !set(to.Index(i), dest, opt.DeepCopy, converters) { + isSet, err := set(to.Index(i), dest, opt.DeepCopy, converters) + if err != nil { + return err + } + if !isSet { // ignore error while copy slice element err = copier(to.Index(i).Addr().Interface(), dest.Interface(), opt) if err != nil { @@ -373,6 +456,31 @@ func copier(toValue interface{}, fromValue interface{}, opt Option) (err error) return } +func getFieldNamesMapping(mappings map[converterPair]FieldNameMapping, fromType reflect.Type, toType reflect.Type) map[string]string { + var fieldNamesMapping map[string]string + + if len(mappings) > 0 { + pair := converterPair{ + SrcType: fromType, + DstType: toType, + } + if v, ok := mappings[pair]; ok { + fieldNamesMapping = v.Mapping + } + } + return fieldNamesMapping +} + +func fieldByNameOrZeroValue(source reflect.Value, fieldName string) (value reflect.Value) { + defer func() { + if err := recover(); err != nil { + value = reflect.Value{} + } + }() + + return source.FieldByName(fieldName) +} + func copyUnexportedStructFields(to, from reflect.Value) { if from.Kind() != reflect.Struct || to.Kind() != reflect.Struct || !from.Type().AssignableTo(to.Type()) { return @@ -392,14 +500,20 @@ func copyUnexportedStructFields(to, from reflect.Value) { } func shouldIgnore(v reflect.Value, ignoreEmpty bool) bool { - if !ignoreEmpty { - return false - } - - return v.IsZero() + return ignoreEmpty && v.IsZero() } +var deepFieldsLock sync.RWMutex +var deepFieldsMap = make(map[reflect.Type][]reflect.StructField) + func deepFields(reflectType reflect.Type) []reflect.StructField { + deepFieldsLock.RLock() + cache, ok := deepFieldsMap[reflectType] + deepFieldsLock.RUnlock() + if ok { + return cache + } + var res []reflect.StructField if reflectType, _ = indirectType(reflectType); reflectType.Kind() == reflect.Struct { fields := make([]reflect.StructField, 0, reflectType.NumField()) @@ -416,11 +530,13 @@ func deepFields(reflectType reflect.Type) []reflect.StructField { } } } - - return fields + res = fields } - return nil + deepFieldsLock.Lock() + deepFieldsMap[reflectType] = res + deepFieldsLock.Unlock() + return res } func indirect(reflectValue reflect.Value) reflect.Value { @@ -438,98 +554,101 @@ func indirectType(reflectType reflect.Type) (_ reflect.Type, isPtr bool) { return reflectType, isPtr } -func set(to, from reflect.Value, deepCopy bool, converters map[converterPair]TypeConverter) bool { - if from.IsValid() { - if ok, err := lookupAndCopyWithConverter(to, from, converters); err != nil { - return false - } else if ok { - return true - } +func set(to, from reflect.Value, deepCopy bool, converters map[converterPair]TypeConverter) (bool, error) { + if !from.IsValid() { + return true, nil + } + if ok, err := lookupAndCopyWithConverter(to, from, converters); err != nil { + return false, err + } else if ok { + return true, nil + } - if to.Kind() == reflect.Ptr { - // set `to` to nil if from is nil - if from.Kind() == reflect.Ptr && from.IsNil() { - to.Set(reflect.Zero(to.Type())) - return true - } else if to.IsNil() { - // `from` -> `to` - // sql.NullString -> *string - if fromValuer, ok := driverValuer(from); ok { - v, err := fromValuer.Value() - if err != nil { - return false - } - // if `from` is not valid do nothing with `to` - if v == nil { - return true - } - } - // allocate new `to` variable with default value (eg. *string -> new(string)) - to.Set(reflect.New(to.Type().Elem())) - } - // depointer `to` - to = to.Elem() - } - - if deepCopy { - toKind := to.Kind() - if toKind == reflect.Interface && to.IsNil() { - if reflect.TypeOf(from.Interface()) != nil { - to.Set(reflect.New(reflect.TypeOf(from.Interface())).Elem()) - toKind = reflect.TypeOf(to.Interface()).Kind() - } - } - if from.Kind() == reflect.Ptr && from.IsNil() { - return true - } - if toKind == reflect.Struct || toKind == reflect.Map || toKind == reflect.Slice { - return false - } - } - - if from.Type().ConvertibleTo(to.Type()) { - to.Set(from.Convert(to.Type())) - } else if toScanner, ok := to.Addr().Interface().(sql.Scanner); ok { - // `from` -> `to` - // *string -> sql.NullString - if from.Kind() == reflect.Ptr { - // if `from` is nil do nothing with `to` - if from.IsNil() { - return true - } - // depointer `from` - from = indirect(from) - } - // `from` -> `to` - // string -> sql.NullString - // set `to` by invoking method Scan(`from`) - err := toScanner.Scan(from.Interface()) - if err != nil { - return false - } - } else if fromValuer, ok := driverValuer(from); ok { + if to.Kind() == reflect.Ptr { + // set `to` to nil if from is nil + if from.Kind() == reflect.Ptr && from.IsNil() { + to.Set(reflect.Zero(to.Type())) + return true, nil + } else if to.IsNil() { // `from` -> `to` - // sql.NullString -> string - v, err := fromValuer.Value() - if err != nil { - return false + // sql.NullString -> *string + if fromValuer, ok := driverValuer(from); ok { + v, err := fromValuer.Value() + if err != nil { + return true, nil + } + // if `from` is not valid do nothing with `to` + if v == nil { + return true, nil + } } - // if `from` is not valid do nothing with `to` - if v == nil { - return true + // allocate new `to` variable with default value (eg. *string -> new(string)) + to.Set(reflect.New(to.Type().Elem())) + } + // depointer `to` + to = to.Elem() + } + + if deepCopy { + toKind := to.Kind() + if toKind == reflect.Interface && to.IsNil() { + if reflect.TypeOf(from.Interface()) != nil { + to.Set(reflect.New(reflect.TypeOf(from.Interface())).Elem()) + toKind = reflect.TypeOf(to.Interface()).Kind() } - rv := reflect.ValueOf(v) - if rv.Type().AssignableTo(to.Type()) { - to.Set(rv) - } - } else if from.Kind() == reflect.Ptr { - return set(to, from.Elem(), deepCopy, converters) - } else { - return false + } + if from.Kind() == reflect.Ptr && from.IsNil() { + return true, nil + } + if _, ok := to.Addr().Interface().(sql.Scanner); !ok && (toKind == reflect.Struct || toKind == reflect.Map || toKind == reflect.Slice) { + return false, nil } } - return true + if from.Type().ConvertibleTo(to.Type()) { + to.Set(from.Convert(to.Type())) + } else if toScanner, ok := to.Addr().Interface().(sql.Scanner); ok { + // `from` -> `to` + // *string -> sql.NullString + if from.Kind() == reflect.Ptr { + // if `from` is nil do nothing with `to` + if from.IsNil() { + return true, nil + } + // depointer `from` + from = indirect(from) + } + // `from` -> `to` + // string -> sql.NullString + // set `to` by invoking method Scan(`from`) + err := toScanner.Scan(from.Interface()) + if err != nil { + return false, nil + } + } else if fromValuer, ok := driverValuer(from); ok { + // `from` -> `to` + // sql.NullString -> string + v, err := fromValuer.Value() + if err != nil { + return false, nil + } + // if `from` is not valid do nothing with `to` + if v == nil { + return true, nil + } + rv := reflect.ValueOf(v) + if rv.Type().AssignableTo(to.Type()) { + to.Set(rv) + } else if to.CanSet() && rv.Type().ConvertibleTo(to.Type()) { + to.Set(rv.Convert(to.Type())) + } + } else if from.Kind() == reflect.Ptr { + return set(to, from.Elem(), deepCopy, converters) + } else { + return false, nil + } + + return true, nil } // lookupAndCopyWithConverter looks up the type pair, on success the TypeConverter Fn func is called to copy src to dst field. @@ -541,7 +660,6 @@ func lookupAndCopyWithConverter(to, from reflect.Value, converters map[converter if cnv, ok := converters[pair]; ok { result, err := cnv.Fn(from.Interface()) - if err != nil { return false, err } @@ -574,7 +692,7 @@ func parseTags(tag string) (flg uint8, name string, err error) { if unicode.IsUpper([]rune(t)[0]) { name = strings.TrimSpace(t) } else { - err = errors.New("copier field name tag must be start upper case") + err = ErrFieldNameTagStartNotUpperCase } } } @@ -651,8 +769,14 @@ func checkBitFlags(flagsList map[string]uint8) (err error) { return } -func getFieldName(fieldName string, flgs flags) (srcFieldName string, destFieldName string) { +func getFieldName(fieldName string, flgs flags, fieldNameMapping map[string]string) (srcFieldName string, destFieldName string) { // get dest field name + if name, ok := fieldNameMapping[fieldName]; ok { + srcFieldName = fieldName + destFieldName = name + return + } + if srcTagName, ok := flgs.SrcNames.FieldNameToTag[fieldName]; ok { destFieldName = srcTagName if destTagName, ok := flgs.DestNames.TagToFieldName[srcTagName]; ok { @@ -686,7 +810,6 @@ func getFieldName(fieldName string, flgs flags) (srcFieldName string, destFieldN } func driverValuer(v reflect.Value) (i driver.Valuer, ok bool) { - if !v.CanAddr() { i, ok = v.Interface().(driver.Valuer) return @@ -695,3 +818,11 @@ func driverValuer(v reflect.Value) (i driver.Valuer, ok bool) { i, ok = v.Addr().Interface().(driver.Valuer) return } + +func fieldByName(v reflect.Value, name string, caseSensitive bool) reflect.Value { + if caseSensitive { + return v.FieldByName(name) + } + + return v.FieldByNameFunc(func(n string) bool { return strings.EqualFold(n, name) }) +} diff --git a/vendor/github.com/jinzhu/copier/errors.go b/vendor/github.com/jinzhu/copier/errors.go index cf7c5e74b5..f50ea32bf0 100644 --- a/vendor/github.com/jinzhu/copier/errors.go +++ b/vendor/github.com/jinzhu/copier/errors.go @@ -3,8 +3,9 @@ package copier import "errors" var ( - ErrInvalidCopyDestination = errors.New("copy destination is invalid") - ErrInvalidCopyFrom = errors.New("copy from is invalid") - ErrMapKeyNotMatch = errors.New("map's key type doesn't match") - ErrNotSupported = errors.New("not supported") + ErrInvalidCopyDestination = errors.New("copy destination must be non-nil and addressable") + ErrInvalidCopyFrom = errors.New("copy from must be non-nil and addressable") + ErrMapKeyNotMatch = errors.New("map's key type doesn't match") + ErrNotSupported = errors.New("not supported") + ErrFieldNameTagStartNotUpperCase = errors.New("copier field name tag must be start upper case") ) diff --git a/vendor/modules.txt b/vendor/modules.txt index d385b56b55..a83012cc5a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -156,7 +156,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.55.1-0.20230816154734-519ed7fea9bd +# github.com/containers/common v0.55.1-0.20230824140149-b27c2ba2b7e1 ## explicit; go 1.18 github.com/containers/common/libimage github.com/containers/common/libimage/define @@ -653,7 +653,7 @@ github.com/hashicorp/go-retryablehttp # github.com/inconshreveable/mousetrap v1.1.0 ## explicit; go 1.18 github.com/inconshreveable/mousetrap -# github.com/jinzhu/copier v0.3.5 +# github.com/jinzhu/copier v0.4.0 ## explicit; go 1.13 github.com/jinzhu/copier # github.com/josharian/intern v1.0.0